Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an option to set 'certificateVerification' to the client builder #980

Merged
merged 1 commit into from
Sep 30, 2020

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Sep 30, 2020

Motivation:

We allow users to set the certificateVerification if they configure
their client directly using ClientConnection.Configuration but not via
the builder API.

Modifications:

  • Add withTLS(certificateVerification:) to the client connection builder.
    (The same option is already available on the server builder)

Result:

Users can set the certificate verification mode on the client builder.

@glbrntt glbrntt added the 🆕 semver/minor Adds new public API. label Sep 30, 2020
@glbrntt glbrntt requested a review from Lukasa September 30, 2020 08:33
@Lukasa
Copy link
Collaborator

Lukasa commented Sep 30, 2020

Worth adding a test for this?

Motivation:

We allow users to set the `certificateVerification` if they configure
their client directly using `ClientConnection.Configuration` but not via
the builder API.

Modifications:

- Add `withTLS(certificateVerification:)` to the client connection builder.
  (The same option is already available on the server builder)

Result:

Users can set the certificate verification mode on the client builder.
@glbrntt glbrntt force-pushed the gb-cert-verification-option branch from 221404b to c3a1915 Compare September 30, 2020 13:02
@glbrntt
Copy link
Collaborator Author

glbrntt commented Sep 30, 2020

Worth adding a test for this?

Done. I was just being lazy 🙈

@Lukasa Lukasa merged commit e2e138d into grpc:main Sep 30, 2020
@glbrntt glbrntt deleted the gb-cert-verification-option branch December 10, 2020 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🆕 semver/minor Adds new public API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants