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

v2: Replace our chaining of interceptors with newest gRPC ones. #342

Closed
bwplotka opened this issue Oct 2, 2020 · 5 comments
Closed

v2: Replace our chaining of interceptors with newest gRPC ones. #342

bwplotka opened this issue Oct 2, 2020 · 5 comments
Labels

Comments

@bwplotka
Copy link
Collaborator

bwplotka commented Oct 2, 2020

See: https://godoc.org/google.golang.org/grpc#ChainUnaryInterceptor

Blocked on upgrade PR: #321

Pulled from #275 for visibility.

Blocker for v2.

@bwplotka bwplotka added the v2 label Oct 2, 2020
@bwplotka bwplotka changed the title Replace our chaining of interceptors with newest gRPC ones. v2: Replace our chaining of interceptors with newest gRPC ones. Oct 2, 2020
@curiousleo
Copy link

Looks like this has been resolved by #385 and can be closed?

@johanbrandhorst
Copy link
Collaborator

Sounds good to me!

@bwplotka
Copy link
Collaborator Author

I haven't seen this fix 😱 Amazing, thanks! Super cool, this brings us almost ready for v2 release (: Thank you so much!

@ecordell
Copy link
Contributor

ecordell commented Oct 18, 2022

grpc's ChainUnaryInterceptor returns a ServerOption, not an interceptor, so it can't be used to build chains that are also interceptors (to be included in other chains).

We use this behavior in spicedb to associate default interceptors with service implementations:

https://github.com/authzed/spicedb/blob/500d9dcbf376a545a96c0d0b62340e7f1c653e85/internal/services/v1/relationships.go#L58-L68

which are then included in bigger interceptor chains for actually serving, based on startup options, etc.

It seems pretty handy to have func(...Interceptor) Interceptor in this library - any chance we could bring it back for v2? Or do you think it would be better to see if grpc-go could have an option to change the return type of the Chain fns?

(We could refactor a bit to avoid this need, but these chain helpers seemed generally useful).

@johanbrandhorst
Copy link
Collaborator

Hi Evan. I think we're trying to remove as much API as possible at this point, since we're cutting a new major. Do you think you could refactor your code or re-implement the old code in your code base? If we get more requests for this we can always add it later.

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

No branches or pull requests

4 participants