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

Deprecate usage of gresolver.Address.Metadata #19706

Open
siyuanfoundation opened this issue Apr 3, 2025 · 5 comments
Open

Deprecate usage of gresolver.Address.Metadata #19706

siyuanfoundation opened this issue Apr 3, 2025 · 5 comments

Comments

@siyuanfoundation
Copy link
Contributor

What would you like to be added?

Deprecate usage of gresolver.Address.Metadata in client/v3/naming/resolver/resolver.go, and add Attributes field.

Why is this needed?

gresolver.Address.Metadata in grpc is deprecated, and should use Attributes instead.
https://github.com/grpc/grpc-go/blob/51d6a43ec59753d42ccd02bb12d2e9e40c164f0f/resolver/resolver.go#L117-L121

@amosehiguese
Copy link

Hi @siyuanfoundation, I'd like to work on implementing this feature. I'll follow the gRPC resolver API changes to deprecate the Metadata usage in favor of Attributes.

@amosehiguese
Copy link

Hi @siyuanfoundation

I just sent in a pr with the implementation in place.

@siyuanfoundation
Copy link
Contributor Author

/assign @amosehiguese

Thank you!

@siyuanfoundation
Copy link
Contributor Author

We should also have use the supported fields in tests like #19702

@amosehiguese
Copy link

amosehiguese commented Apr 4, 2025

Yeah, I noticed the tests still uses the metadata field, so my first attempt was to introduce the new field and make it backward compatible.

However, now that you've mentioned the Metadata field should remain in the Endpoint, I will revert back to that only and handle the conversion process underneath. If this is fine, please signify.

Lastly, I will love to mention that creating a new Attribute with the attributes.New(k, v any) to be used in the gresolver.Address Attribute field, takes in a key and a value as opposed to a single "any" argument(i.e the metadata passed)

Would you say it's okay to use the up.Key for the key and the up.Endpoint.Metadata for the value?

@ivanvc ivanvc removed the help wanted label Apr 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants