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

docs/customizingyourgateway: add ?pretty example #954

Merged

Conversation

srenatus
Copy link
Contributor

As discussed in slack. The code is rough, but should illustrate the
point.

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.

Thanks for this! I think it's a great example of how easy the gateway is to configure.

docs/_docs/customizingyourgateway.md Outdated Show resolved Hide resolved
docs/_docs/customizingyourgateway.md Outdated Show resolved Hide resolved
docs/_docs/customizingyourgateway.md Show resolved Hide resolved
prettier := func(h http.Handler) http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
defer h.ServeHTTP(w, r)
// if the "Accept" is set and NOT */* (sent by curl), keep it as-is
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why does this need special casing around */*?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't, in general. However, all folks I know (small sample) would end up using curl on that endpoint, and curl sends Accept: */* by default. So, if we only check that an Accept header was not provided, this will not work for curl users.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If figured, if you do not care (i.e. "accept */*"), we might a well override your choice 😄

Copy link
Collaborator

@johanbrandhorst johanbrandhorst Jun 18, 2019

Choose a reason for hiding this comment

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

What about removing this check altogether? Any Accept header could be overridden when the pretty parameter is present, maybe? What cases would that mess with?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh you're right. In this example, that's surely fine. I was thinking better safe than sorry; but it's fine here.

@johanbrandhorst
Copy link
Collaborator

I think the bazel failures are because there's an update available - I'll get @drigz or @achew22 on sorting that (#955), we can still merge this change.

As discussed in slack. The code is rough, but should illustrate the
point.

Signed-off-by: Stephan Renatus <[email protected]>
@johanbrandhorst johanbrandhorst merged commit ee2f385 into grpc-ecosystem:master Jun 18, 2019
@johanbrandhorst
Copy link
Collaborator

Thanks for your contribution!

@srenatus srenatus deleted the sr/docs/add-pretty-example branch June 18, 2019 14:12
adasari pushed a commit to adasari/grpc-gateway that referenced this pull request Apr 9, 2020
As discussed in slack. The code is rough, but should illustrate the
point.

Signed-off-by: Stephan Renatus <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants