client: bypass endpoint name resolution #477
Conversation
internal/client/client.go
Outdated
| @@ -74,7 +75,7 @@ func newClientForContext(cmd *cobra.Command, contextName string, secretStore sto | |||
| return nil, err | |||
| } | |||
|
|
|||
| return authzed.NewClient(token.Endpoint, dialOpts...) | |||
| return authzed.NewClient(withPassthroughTarget(token.Endpoint), dialOpts...) | |||
There was a problem hiding this comment.
I'm hesitant about the way that this changes the behavior for all calls. Ideally we'd only include this behavior when explicitly using a proxy. Is there a way to rework this so that we only apply it when the proxy is defined?
There was a problem hiding this comment.
My current reading/understanding is that if you use the dns resolver, which is the default, it attempts to do DNS resolution before creating connections, which allows it to get a list of available hosts for client-side load balancing. Otherwise it's deferring the resolution to the lib underlying the gRPC client, which means it will only have a single target and a single connection.
That may be sufficient given that zed is a CLI utility, and it doesn't have the same performance requirements as (say) the dispatcher within SpiceDB.
I think I'd still prefer to minimize the possibility of surprising behaviors, which would point to leaving the default intact in as many cases as possible.
There was a problem hiding this comment.
sounds about right, changing this logic to be employed only when a proxy is specified.
kartikaysaxena
force-pushed
the
passthrough-endpoint
branch
2 times, most recently
from
fe0fde1 to
bac042c
Compare
Signed-off-by: Kartikay <[email protected]>
kartikaysaxena
force-pushed
the
passthrough-endpoint
branch
from
bac042c to
4a4d8ce
Compare
Should fix #475
Works as per this (tested locally with socks5 proxy)