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

vendored package breaks public API #105

Closed
GeertJohan opened this issue Nov 21, 2017 · 8 comments · Fixed by #109
Closed

vendored package breaks public API #105

GeertJohan opened this issue Nov 21, 2017 · 8 comments · Fixed by #109

Comments

@GeertJohan
Copy link

GeertJohan commented Nov 21, 2017

It looks like google.golang.org/grpc/metadata is now being vendored by this project. This breaks the public API of this package, which might not be your intention?

I don't believe google.golang.org/grpc/metadata needs to be vendored by this package. It kinda breaks type compatibility between this package and other packages using metadata.

somefile.go:74:46: cannot use stream (type "google.golang.org/grpc".ServerStream) as type "github.com/grpc-ecosystem/go-grpc-middleware/vendor/google.golang.org/grpc".ServerStream in argument to grpc_middleware.WrapServerStream:
        "google.golang.org/grpc".ServerStream does not implement "github.com/grpc-ecosystem/go-grpc-middleware/vendor/google.golang.org/grpc".ServerStream (wrong type for SendHeader method)
                have SendHeader("google.golang.org/grpc/metadata".MD) error
                want SendHeader("github.com/grpc-ecosystem/go-grpc-middleware/vendor/google.golang.org/grpc/metadata".MD) error
@loderunner
Copy link

I have the same issue with interceptors. It is now impossible to mix-and-match grpc_middleware interceptors with grpc interceptors.

server/serve.go:143:37: cannot use i (type "google.golang.org/grpc".UnaryServerInterceptor) as type "github.com/grpc-ecosystem/go-grpc-middleware/vendor/google.golang.org/grpc".UnaryServerInterceptor in argument to grpc_middleware.ChainUnaryServer
server/serve.go:143:37: cannot use grpc_middleware.ChainUnaryServer(i) (type "github.com/grpc-ecosystem/go-grpc-middleware/vendor/google.golang.org/grpc".UnaryServerInterceptor) as type "google.golang.org/grpc".UnaryServerInterceptor in argument to "google.golang.org/grpc".UnaryInterceptor

@Kilmar
Copy link

Kilmar commented Nov 21, 2017

Same problem here but with Context

have Context() ".../vendor/golang.org/x/net/context".Context
want Context() ".../vendor/github.com/grpc-ecosystem/go-grpc-middleware/vendor/golang.org/x/net/context".Context

@chadnickbok
Copy link

Oh lawdy please remove these vendored packages. Checking in a lockfile is one thing, but checking in the entire vendor directory is Not The Way.

@GeertJohan
Copy link
Author

@domgreen could you please look into this?

@GeertJohan
Copy link
Author

Preferably the master branch is rolled back to 2e3f98d. Afterwards re-apply the commits added since #101 which are: b5293e3 and 952ab2f. This cleans up the ~1.8 milion changes introduced in ab28732.

@domgreen
Copy link
Contributor

@GeertJohan will be looking at this this morning

@yifanzz
Copy link
Contributor

yifanzz commented Nov 22, 2017

Merged @domgreen's fix. Let us know if this fixes your issue. @GeertJohan

@GeertJohan
Copy link
Author

Looks like the problem has been solved indeed.
Thanks @domgreen @yifanzz

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

Successfully merging a pull request may close this issue.

6 participants