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
xds: add new fields to XDSConfig #3100
Conversation
63e2717
to
d5e08e8
Compare
9071527
to
0db4b77
Compare
scpb "google.golang.org/grpc/internal/proto/grpc_service_config" | ||
) | ||
|
||
// TestExampleMarshalToJSON is an example to print json format of xds_config. |
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.
Name doesn't match. I thought this was going to be an example without verification?
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.
Name fixed.
Examples don't output anything. Tests can print the json (to be copied or modified).
xds/internal/balancer/config.go
Outdated
// When unmarshalling, we iterate through the childPolicy/fallbackPolicy lists | ||
// and select the first LB policy which has been registered. | ||
func (l *XDSConfig) UnmarshalJSON(data []byte) error { | ||
var val map[string]json.RawMessage |
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.
Optional, may not be worth it: would it be easier to define the literal struct, Unmarshal into it, and then post-process:
type xdsConfigJSON struct {
BalancerName string
ChildPolicy []*loadBalancingConfig
FallbackPolicy []*loadBalancingConfig
EDSServiceName string
LRSLoadReportingServerName string
}
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.
This is a bit cleaner. Done
xds/internal/balancer/config.go
Outdated
|
||
// MarshalJSON returns a JSON encoding of l. | ||
func (l *XDSConfig) MarshalJSON() ([]byte, error) { | ||
return nil, nil |
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.
Should this return an unimplemented error or grpclog.Fatal() instead?
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.
Return an error now.
[xds_service_config_new_fields] fix underscore in package name [xds_service_config_new_fields] misspell [xds_service_config_new_fields] 1 [xds_service_config_new_fields] use xdsconfig.EdsServiceName if not empty
0db4b77
to
da20f2e
Compare
d126389
to
a9777d3
Compare
Fields are added in: grpc/grpc-proto#64
Other changes: