-
Notifications
You must be signed in to change notification settings - Fork 213
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
Accept URIs with scheme, add default SSL options for https, deprecate URLs without scheme #357
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… URLs without scheme This commit changes how addresses are handled by GRPC.Stub.connect/2. First, by accepting URIs with scheme. Before, `http://localhost:50051" would cause a crash. Second, by using the URI scheme to automatically set a GRPC credential if no GRPC credentials were provided. If we were to remove this part, we could at least validate that a GRPC credential was provided when the scheme is https. Finally, by deprecating receiving URLs without scheme, such as `localhost:50051`. This isn't strictly necessary, but in my opinion makes things more consistent when we want to support both local sockets and remote urls.
polvalente
reviewed
Apr 5, 2024
polvalente
reviewed
Apr 5, 2024
polvalente
reviewed
Apr 5, 2024
polvalente
reviewed
Apr 5, 2024
Co-authored-by: Paulo Valente <16843419+polvalente@users.noreply.github.com>
Pushed a few commits doing (I believe) all suggestions :) |
polvalente
reviewed
Apr 5, 2024
polvalente
reviewed
Apr 5, 2024
polvalente
approved these changes
Apr 5, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only 1 question left :)
Makes all examples in the documentation include `http` or `https`. Changes tests to avoid using deprecated code.
This reverts commit ec02a94.
polvalente
reviewed
Apr 6, 2024
polvalente
reviewed
Apr 6, 2024
polvalente
reviewed
Apr 6, 2024
polvalente
reviewed
Apr 6, 2024
polvalente
approved these changes
Apr 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit changes how addresses are handled by GRPC.Stub.connect/2.
First, by accepting URIs with scheme. Before, `http://localhost:50051" would
cause a crash.
Second, by using the URI scheme to automatically set a GRPC credential if no
GRPC credentials were provided. If we were to remove this part, we could at
least validate that a GRPC credential was provided when the scheme is https.
Finally, by deprecating receiving URLs without scheme, such aslocalhost:50051
.This isn't strictly necessary, but in my opinion makes things more consistent when
we want to support both local sockets and remote urls.
Edit: the deprecation part was removed after review.