-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Why is the service config passed as a JSON-String just to get converterted to a struct anyway? #7275
Comments
@DerArkeN Thanks for raising this question! JSON representation has advantage over the struct/protobuf representation, which is that it is possible to encode configurations for LB policies that are not known to gRPC. In protobuf form, the loadBalancingConfig field contains a oneof supporting only the built-in LB policies. However, in JSON form, the field inside the oneof is encoded as a string that indicates the LB policy name. In JSON form, that string can be any arbitrary value, not just one of the supported policies inside of the oneof, so third-party policies can be selected. For more info, please check this out. |
@aranjans Thank you for your answer! This definitely makes sense, I am aware of the issues with oneof using protobuf as well. Though I think it would be helpful to provide a struct which represents the JSON tags for the method config since they are not arbitrary (at least I think so). When implementing my retryPolicy I spend a lot of time looking up the field names and making sure the passed values are in the correct format. type ServiceConfig struct {
MethodConfig []*MethodConfig `json:"methodConfig"`
LoadBalancingConfigJSON []string `json:"loadBalancingConfig"`
}
type MethodConfig struct {
Name []*Name `json:"name"`
WaitForReady bool `json:"waitForReady"`
RetryPolicy *RetryPolicy `json:"retryPolicy"`
}
type Name struct {
// the service name from the proto file (package.service)
Service string `json:"service"`
Method string `json:"method"`
}
type RetryPolicy struct {
MaxAttempts int `json:"MaxAttempts"`
InitialBackoff string `json:"InitialBackoff"`
MaxBackoff string `json:"MaxBackoff"`
... This is just my personal preference though and I don't know how much sense It would make to imlement this in practice since I don't know enough about this topic... Thank you for your time! |
Could you please shed some more light on how (and why) you are specifying the retry configuration on the client, and not getting it from the name resolver? Thanks. |
Hello, I looked at this example . |
Hmm ... we probably have to revisit that example. Did you get a chance to look at https://github.com/grpc/grpc/blob/master/doc/service_config.md, where in the first line it says: The service config is a mechanism that allows service owners to publish parameters to be automatically used by all clients of their service. Is pushing the service config through the name resolver not an option for you? |
I had a discussion about service configs with the extended gRPC team and based on that, here are some thoughts/ideas:
Also, with regards to changing the API to support accepting service config as a struct (with json tags), our preference is to accept service config as JSON in our API, since that helps to keep our API as simple as it can be. While we are not totally opposed to the idea of supporting other means of accepting service config through our APIs, it is not a priority for us, and unless it is something that multiple users would want, we are happy with what we have currently. Hope this helps. |
Hello once again, thank you for the detailed answer and the effort you put into it, it helped a lot! |
I am using the grpc retry policy for some of my services. I don't want to use a raw JSON string in my code and therefore made a struct with JSON tags which I later Marshal into the json string that I pass to the dialOption grpc.WithDefaultServiceConfig.
I looked into the code just to realize there already exists a struct for the service config which justs get's filled by the JSON string anyway?
Why is the method signature for grpc.WithDefaultServiceConfig not
?
Thank you in advance.
The text was updated successfully, but these errors were encountered: