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

feat: add mux middlewares and inject http pattern #4290

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nikitaksv
Copy link

@nikitaksv nikitaksv commented May 6, 2024

References to other Issues or PRs

Issue #4319

Have you read the Contributing Guidelines?

Yes

Brief description of what is fixed or changed

Added Middleware for ServerMux
Added the option of passing a HTTP Pattern to the request context

Other comments

Our team encountered a problem that middleware can only be applied to the Mux "http.Handler", which is unsafe for our application. Middleware is triggered regardless of whether a route exists or not, which leads to unnecessary triggering of middlewares. Also in http.Handler middleware there is no way except "defer" to get http pattern, so we added an option to pass HttpPattern in the request context before triggering middlewares.

@nikitaksv nikitaksv marked this pull request as ready for review May 6, 2024 09:31
@johanbrandhorst
Copy link
Collaborator

Hi, thanks for your PR. However, please open an issue to discuss the problem you're having before submitting a PR. I'm not convinced this is functionality we want, so please open a PR to discuss your specific problem first. Thanks!

Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I'm happy to go ahead with this feature. I would request that you update the comment on the generated direct-to-implementation registration functions in template.go:

// Note: the gRPC framework executes interceptors within the gRPC handler. If the passed in "{{$svc.InstanceName}}Client"
// doesn't go through the normal gRPC flow (creating a gRPC client etc.) then it will be up to the passed in
// "{{$svc.InstanceName}}Client" to call the correct interceptors.

to nudge people towards the use of this middleware functionality as an alternative to gRPC interceptors.

runtime/mux.go Outdated Show resolved Hide resolved
runtime/mux.go Outdated Show resolved Hide resolved
runtime/mux_test.go Show resolved Hide resolved
runtime/mux.go Outdated Show resolved Hide resolved
nikitaksv and others added 3 commits May 16, 2024 17:28
Copy link
Collaborator

@johanbrandhorst johanbrandhorst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@johanbrandhorst
Copy link
Collaborator

Looks like we need to regenerate the files after the update?

@nikitaksv
Copy link
Author

nikitaksv commented May 21, 2024

Looks like we need to regenerate the files after the update?

It is not possible to regenerate files. Inside make generate, buf build is used, which has blocked its server for my country :(
Could you restore the files yourself?

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 this pull request may close these issues.

None yet

2 participants