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

bufnet not supported by NewClient() ? #7091

Closed
gnuletik opened this issue Apr 4, 2024 · 5 comments
Closed

bufnet not supported by NewClient() ? #7091

gnuletik opened this issue Apr 4, 2024 · 5 comments

Comments

@gnuletik
Copy link

gnuletik commented Apr 4, 2024

With the v1.63.0 release, I tried to migrate code that uses the, now deprected, DialContext function to NewClient().

However, when using NewClient with the bufnet protocol (https://pkg.go.dev/google.golang.org/grpc/test/bufconn), my test fails.

Here's the change:

-       conn, err := grpc.DialContext(
-               ctx, "bufnet",
+       conn, err := grpc.NewClient(
+               "bufnet",
                grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) {
                        return lis.DialContext(ctx)
                }),

The test fails with:

rpc error: code = Unavailable desc = dns: A record lookup error: lookup bufnet on 127.0.0.53:53: server misbehaving

How can I use bufnet with the NewClient function?

@SLoeuillet
Copy link

Default resolver is now dns with NewClient, was passthrough
I suppose you'll have to force passthrough resolver

With WithResolvers ?

@SLoeuillet
Copy link

SLoeuillet commented Apr 4, 2024

From DialContext 1.63.0 doc :

One subtle difference between NewClient and Dial and DialContext is that the former uses "dns" as the default name resolver, while the latter use "passthrough" for backward compatibility. This distinction should not matter to most users, but could matter to legacy users that specify a custom dialer and expect it to receive the target string directly.

@gnuletik
Copy link
Author

gnuletik commented Apr 4, 2024

Thanks for the feedback @SLoeuillet!

I've gave a check to WithResolvers() (which is experimental). It expects a resolver.Builder.

Ideally, I'd pass a passthrough's Builder in order to get the original behavior.
But the passthrough package is only available behind an internal/ directory, so I can't import it:

package passthrough
import _ "google.golang.org/grpc/internal/resolver/passthrough" // import for side effects after package was moved

Initially, I also checked if I could set the defaultScheme to passthrough (that's what grpc.DialContext does now).
But the field and the setter are are not exported either.

grpc-go/dialoptions.go

Lines 662 to 668 in c68f456

// withDefaultScheme is used to allow Dial to use "passthrough" as the default
// name resolver, while NewClient uses "dns" otherwise.
func withDefaultScheme(s string) DialOption {
return newFuncDialOption(func(o *dialOptions) {
o.defaultScheme = s
})
}

The only workaround I think of would be to copy the passthrough implementation, so I can pass it to WithResolvers() but that seems counter-intuitive.

As of now, in order to upgrade grpc-go to v1.63.0, I think the best way is to keep using the deprecated grpc.DialContext() function.

Is it intended by the grpc-go developer team? Is there an alternative?

Thanks!

@dfawley
Copy link
Member

dfawley commented Apr 4, 2024

You can specify the passthrough resolver by using "passthrough:///bufnet" as your target instead (and actually buffnet is arbitrary; you could put anything there instead, or probably omit it entirely).

Or you can globally make the default resolver "passthrough" by using https://pkg.go.dev/google.golang.org/grpc/resolver#SetDefaultScheme -- I do not recommend this approach, though, as "passthrough" has some negative properties and really only exists for backward compatibility reasons.

You can also feel free to keep using Dial; it is not going anywhere. We don't recommend its use, but we will continue to support it for all 1.x releases (and no 2.x release is being considered).

Ideally we will eventually add support for an "in-memory" transport for your exact use case. However, that does not currently exist. That would probably require something like #906 which we just don't have the resources to prioritize.

@dfawley dfawley closed this as completed Apr 4, 2024
@gnuletik
Copy link
Author

gnuletik commented Apr 4, 2024

Oh awesome! It works like a charm!
Thanks a lot!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants