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

Support colon in final path segment, last match wins behavior (behind flags) #949

Merged
merged 2 commits into from
Jun 16, 2019

Conversation

jfhamlin
Copy link
Contributor

Addresses #224. Reintroduces changes from #708, this time behind a command line flag (-allow_colon_final_segments) and adds a new option to runtime.ServeMux to enable "last match wins" behavior. Please see discussion in #224 and #708 for context.

@johanbrandhorst
Copy link
Collaborator

Hi @jfhamlin, thanks for the contribution! Looks like you need to regenerate some of the example files, please see CONTRIBUTING.md for two docker one liners you can run.

Signed-off-by: James Hamlin <[email protected]>
@jfhamlin
Copy link
Contributor Author

please see CONTRIBUTING.md for two docker one liners you can run.

Done!

@codecov-io
Copy link

Codecov Report

Merging #949 into master will increase coverage by 0.17%.
The diff coverage is 82.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #949      +/-   ##
==========================================
+ Coverage   53.06%   53.23%   +0.17%     
==========================================
  Files          40       40              
  Lines        3969     3999      +30     
==========================================
+ Hits         2106     2129      +23     
- Misses       1667     1672       +5     
- Partials      196      198       +2
Impacted Files Coverage Δ
protoc-gen-grpc-gateway/descriptor/registry.go 58.77% <0%> (-1.05%) ⬇️
runtime/mux.go 55.38% <100%> (+2.15%) ⬆️
protoc-gen-grpc-gateway/gengateway/template.go 67.03% <100%> (+1.51%) ⬆️
runtime/pattern.go 90.38% <87.5%> (-1.05%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cddead4...d11e56d. Read the comment docs.

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, thanks for this brilliant contribution!

@johanbrandhorst johanbrandhorst merged commit e77cc68 into grpc-ecosystem:master Jun 16, 2019
@jfhamlin jfhamlin deleted the f/colon-path-segment branch June 17, 2019 16:09
@aysylu
Copy link

aysylu commented Jun 17, 2019

This appears to be breaking existing projects with:

proto/v1beta1/project_go_proto/project.pb.gw.go:244:75: too many arguments in call to runtime.NewPattern
proto/v1beta1/project_go_proto/project.pb.gw.go:244:135: undefined: runtime.AssumeColonVerbOpt

Test link: https://circleci.com/gh/grafeas/grafeas/1066?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link. Could you provide some insight into what's broken?

@johanbrandhorst
Copy link
Collaborator

@aysylu I've just released 1.9.2, which should fix your build. As a general note, the error you're getting is because you're using the master version of the generator (protoc-gen-grpc-gateway), and a locked version of the runtime. Ideally, these should always be the same version. In reality, we know this confuses a lot of users, so please update to 1.9.2 and it should fix your issue.

@aysylu
Copy link

aysylu commented Jun 17, 2019

@johanbrandhorst we were able to get around the issue updating our config and pointing to 1.9.0. We did test whether 1.9.2 and it looks like it fails with the same issue, just an FYI. Happy to provide more information on the dependencies if it's helpful.

@johanbrandhorst
Copy link
Collaborator

Hm, how are you generating the files and versioning the runtime? I'd be very interested in hearing of 1.9.2 is broken.

adasari pushed a commit to adasari/grpc-gateway that referenced this pull request Apr 9, 2020
… flags) (grpc-ecosystem#949)

* Support colon in final path segment, last match wins behavior (behind flags)

Signed-off-by: James Hamlin <[email protected]>

* Update examples

Signed-off-by: James Hamlin <[email protected]>

Fixes grpc-ecosystem#224
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.

5 participants