Introduce custom NameResolver.Args #11669
Merged
Conversation
This file contains hidden or 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
jdcormie
added
the
kokoro:force-run
grpc-kokoro
removed
the
kokoro:force-run
Member
Author
|
Friendly 1 week ping. This is the domain-specific Key<>-based approach you requested in our last meeting. |
ejona86
reviewed
Nov 13, 2024
Member
ejona86
left a comment
ejona86
left a comment
There was a problem hiding this comment.
I like how Extensions was implemented to avoid copying the map on build().
Fully remove Args.Extensions from the public API (@internal) Make TODO non-javadoc google-java-format unused imports
Member
Author
👍 I can't take credit though, just copy pasted from Attributes. LMK if you think one of the other special-purpose containers would be a better fit. |
Update NameResolverTest. Include setAll() coverage
Member
Author
|
Friendly ping :) |
Member
Author
|
Friendly 2 week ping 🙂 |
ejona86
reviewed
Dec 9, 2024
Member
ejona86
left a comment
ejona86
left a comment
There was a problem hiding this comment.
Sending what I have. It is proof at least I looked at this today.
Member
Author
|
Gentle weekly ping |
Call these "custom" args rather than extensions for consistency with CallOptions.
jdcormie
changed the title
Member
Author
|
NameResolver.Args.Extensions no longer exists. Call these "custom" args to match CallOptions terminology. PTAL |
ejona86
approved these changes
Dec 20, 2024
Member
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
3 participants
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.
grpc-binder's upcoming
IntentNameResolverneeds to know the target Android user so it can resolve target URIs in the correct place. Unfortunately, Android's built in intent:// URI scheme has no way to specify a user and in fact theandroid.os.UserHandleobject can't reasonably be encoded as a String at all.We solve this problem by extending
NameResolver.Argsto permit externally defined arguments using the same type-safe and domain-specificKey<T>pattern used byCallOptions,ContextandCreateSubchannelArgs. New "custom" arguments could apply to all NameResolvers of a certain URI scheme, to all NameResolvers producing a particular type ofjava.net.SocketAddress, or even to a specificNameResolversubclass.