grpc: do not use balancer attributes during address comparison #6439
Conversation
attributes/attributes.go
Outdated
| if str, ok := k.(interface{ String() string }); ok { | ||
| key = str.String() | ||
| } else if str, ok := k.(string); ok { | ||
| key = str | ||
| } |
There was a problem hiding this comment.
Sad that string doesn't implement Stringer 😞
To consolidate these code paths, and to avoid printing literally ", " if key & val are both not string-y, maybe we should have something like this?:
func str(x interface{}) string {
if v, ok := x.(fmt.Stringer); ok {
return v.String()
} else if v, ok := x.(string); ok {
return v
}
return fmt.Sprintf("<%p>", x)
}There was a problem hiding this comment.
Done. Had to add a "%q" format string to ensure that the key and value strings are quoted.
| Network string | ||
| Address string | ||
| Target string | ||
| Listener net.Listener |
There was a problem hiding this comment.
This is not a parameter for Listen or Dial. Can you document this please and indicate Network & Address aren't used if this is set, and that Target is the responsibility of the user to set properly when Listener is used?
There was a problem hiding this comment.
My code search reveals that nobody is setting Network, Address or Target.
Address is used as a read-only field by numerous tests to retrieve the address of the stub server. Should we just get rid of Network and Target and make Address as a read-only field, or make it a getter method? (Can do in a follow up PR)
There was a problem hiding this comment.
Will do the cleanup in a follow up PR. Thanks.
| } else if v, ok := x.(string); ok { | ||
| return fmt.Sprintf("%q", v) | ||
| } | ||
| return fmt.Sprintf("<%p>", x) |
There was a problem hiding this comment.
This one doesn't print literal quotes.
Why not keep it the way it was? Have this return the actual string and quote it when it's used?
Summary of changes:
net.Listenerto thestubserverFixes internal issue: b/290272899
RELEASE NOTES: