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

Fix #504 Missing Definitions #505

Merged
merged 1 commit into from
Dec 23, 2017

Conversation

warmans
Copy link
Contributor

@warmans warmans commented Dec 15, 2017

This apparently fixes the issue, but I'm not 100% sure why. It seems to be something to do with the refs map missing entries for nested request types.

fixes #504

@achew22
Copy link
Collaborator

achew22 commented Dec 17, 2017

This looks really good and I think it's 100% the fix. Thanks for contributing!

Do you think you could add an example in https://github.com/grpc-ecosystem/grpc-gateway/blob/master/examples/examplepb/a_bit_of_everything.proto that exhibits this problem (and the solution) so that we don't regresson this in the future? Thanks so much!

@warmans
Copy link
Contributor Author

warmans commented Dec 18, 2017

Hi, added something to the example which shows the problem (if the fix is reverted anyway). Let me know if anything is missing.

edit: erm, not sure how to make the tests run in CI. It imports the examples from the grpc-ecosystem path but my changes are only available in my own namespace.

@achew22
Copy link
Collaborator

achew22 commented Dec 20, 2017

edit: erm, not sure how to make the tests run in CI. It imports the examples from the grpc-ecosystem path but my changes are only available in my own namespace.

I'm not sure I understand what you're saying here. Can you help me understand a little better?

@warmans warmans force-pushed the bug-missing-definitions branch from 0e1f743 to 00e5a4f Compare December 21, 2017 08:33
@warmans
Copy link
Contributor Author

warmans commented Dec 21, 2017

Yeah sure, AFAIK the travis tests fail because the new GetWithBody method is undefined on the server in github.com/grpc-ecosystem/grpc-gateway/examples/server. But I cannot implement this method because I cannot use the request type from the imported package:

"github.com/grpc-ecosystem/grpc-gateway/examples/examplepb"

it's only available in

"github.com/warmans/grpc-gateway/examples/examplepb"

But I shouldn't important that. Additionally it looks like travis just go gets the grpc-ecosystem package so I don't know if I could import it even if I wanted to.

This wouldn't be an issue if the PR was a branch rather than a fork so maybe a maintainer could just copy the branch and replace this PR. Or perhaps there is another solution. Relative imports or something. I'm not sure :/

@achew22
Copy link
Collaborator

achew22 commented Dec 22, 2017

I think those errors are probably only occurring in your local $GOROOT. What happens if you mv the $GOROOT/src/github.com/warmans/grpc-gateway directory into $GOROOT/src/github.com/grpc-ecosystem/grpc-gateway. Do you get different errors?

@warmans warmans force-pushed the bug-missing-definitions branch 3 times, most recently from c120f08 to fa6bda2 Compare December 23, 2017 12:13
@warmans
Copy link
Contributor Author

warmans commented Dec 23, 2017

Hah oh god. Why did I not think of that. Should be fixed now.

@warmans warmans force-pushed the bug-missing-definitions branch from 4f7f26b to e7e8908 Compare December 23, 2017 13:28
@achew22 achew22 merged commit 48e4bd9 into grpc-ecosystem:master Dec 23, 2017
adasari pushed a commit to adasari/grpc-gateway that referenced this pull request Apr 9, 2020
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.

protoc-gen-swagger missing definition issue
2 participants