Add 'License' message to the annotation proto. #644

ensonic · 2018-05-08T07:34:12Z

This information is used by the default html doc generator and so it is nice
to be able to provide it.

ensonic · 2018-05-08T07:56:39Z

Just realize this is only the first step, we track down, what needs to be done to make this appear in the final json too.

ensonic · 2018-05-08T08:11:32Z

Now it should be good to go.

ensonic · 2018-05-08T08:25:19Z

Hi, I only get the bazel build to work.
When I try to build with make, I get errors:
https://gist.github.com/ensonic/173b3cfb6032ee730320729d356f8534

achew22 · 2018-05-08T18:15:19Z

protoc-gen-swagger/genswagger/template.go

@@ -784,6 +784,17 @@ func applyTemplate(p param) (string, error) {
 					s.Info.Contact.Email = spb.Info.Contact.Email
 				}
 			}
+			if spb.Info.License != nil {
+				if s.Info.License == nil {
+					s.Info.License = &swaggerLicenseObject{}


I think you need to add the License field to the Info object. I'm surprised this isn't a compile time error

Shouldn't that be generated from "message Info" define in openapiv2.proto? Also when I build it with bazel it actually works. I am getting the license fields in the generated json.

ensonic · 2018-05-11T10:07:05Z

Update when I run 'make test' localy I get:

go test -race github.com/grpc-ecosystem/grpc-gateway/...
warning: ignoring symlink /usr/local/google/home/ensonic/go/src/github.com/grpc-ecosystem/grpc-gateway/bazel-bin
warning: ignoring symlink /usr/local/google/home/ensonic/go/src/github.com/grpc-ecosystem/grpc-gateway/bazel-genfiles
warning: ignoring symlink /usr/local/google/home/ensonic/go/src/github.com/grpc-ecosystem/grpc-gateway/bazel-grpc-gateway
warning: ignoring symlink /usr/local/google/home/ensonic/go/src/github.com/grpc-ecosystem/grpc-gateway/bazel-out
warning: ignoring symlink /usr/local/google/home/ensonic/go/src/github.com/grpc-ecosystem/grpc-gateway/bazel-testlogs
# google.golang.org/grpc/grpclb/grpc_lb_v1/messages
../../../google.golang.org/grpc/grpclb/grpc_lb_v1/messages/messages.pb.go:59:30: undefined: proto.InternalMessageInfo
../../../google.golang.org/grpc/grpclb/grpc_lb_v1/messages/messages.pb.go:112:31: undefined: proto.InternalMessageInfo
../../../google.golang.org/grpc/grpclb/grpc_lb_v1/messages/messages.pb.go:160:40: undefined: proto.InternalMessageInfo
../../../google.golang.org/grpc/grpclb/grpc_lb_v1/messages/messages.pb.go:302:47: undefined: proto.InternalMessageInfo
../../../google.golang.org/grpc/grpclb/grpc_lb_v1/messages/messages.pb.go:358:33: undefined: proto.InternalMessageInfo
../../../google.golang.org/grpc/grpclb/grpc_lb_v1/messages/messages.pb.go:441:41: undefined: proto.InternalMessageInfo
../../../google.golang.org/grpc/grpclb/grpc_lb_v1/messages/messages.pb.go:590:48: undefined: proto.InternalMessageInfo
../../../google.golang.org/grpc/grpclb/grpc_lb_v1/messages/messages.pb.go:639:32: undefined: proto.InternalMessageInfo
../../../google.golang.org/grpc/grpclb/grpc_lb_v1/messages/messages.pb.go:698:28: undefined: proto.InternalMessageInfo
ok  	github.com/grpc-ecosystem/grpc-gateway/codegenerator	0.541s
# github.com/grpc-ecosystem/grpc-gateway/examples/clients/abe
examples/clients/abe/examplepb_a_bit_of_everything.go:47:13: undefined: ExamplepbNumericEnum
examples/clients/abe/examplepb_a_bit_of_everything.go:63:22: undefined: ExamplepbNumericEnum
examples/clients/abe/examplepb_a_bit_of_everything.go:73:22: undefined: ExamplepbNumericEnum
FAIL	github.com/grpc-ecosystem/grpc-gateway/examples/integration [build failed]
ok  	github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway/descriptor	0.569s
ok  	github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway/gengateway	0.590s
ok  	github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway/httprule	0.556s
ok  	github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger	0.549s
ok  	github.com/grpc-ecosystem/grpc-gateway/protoc-gen-swagger/genswagger	0.554s
FAIL	github.com/grpc-ecosystem/grpc-gateway/runtime [build failed]
ok  	github.com/grpc-ecosystem/grpc-gateway/utilities	0.549s
Makefile:169: recipe for target 'test' failed
make: *** [test] Error 2
``
Looks like https://github.com/google/protobuf/issues/4582. Will investigate.
In any case those are different than what travis reported and don't seem to be related to my change.

ensonic · 2018-05-16T15:57:32Z

Hi, I really need help to understand why the tests fail. What exactly should I run locally to build/run not using bazel. I have the git rooted in my gopath:
/home/ensonic/go/src/github.com/grpc-ecosystem/grpc-gateway

achew22 · 2018-05-17T02:12:42Z

Did you try regenerating the openapiv2.pb.go file that is generated from the proto? It looks like we don't have an entry in the Makefile for it. Would you mind adding a new line in the $(SWAGGER_PLUGIN) section of the makefile to invoke protoc like:

protoc -I $(PROTOC_INC_PATH) -I. --plugin=$(GO_PLUGIN) --go_out=$(PKGMAP),plugins=grpc:$(OUTPUT_DIR) $(PROTOFILE)

ensonic · 2018-05-17T10:21:57Z

No, there is the dependent rule
$(OPENAPIV2_GO): $(OPENAPIV2_PROTO) $(GO_PLUGIN)
protoc -I $(PROTOC_INC_PATH) --plugin=$(GO_PLUGIN) -I. --go_out=$(PKGMAP):$(GOPATH)/src $(OPENAPIV2_PROTO)

which builds protoc-gen-swagger/options/openapiv2.proto protoc-gen-swagger/options/annotations.proto and produces the respective '.pb.go' files.

make protoc-gen-swagger/options/openapiv2.pb.go
make: 'protoc-gen-swagger/options/openapiv2.pb.go' is up to date.

I've fixed one test by updating proto-gen-go (go get -u github.com/golang/protobuf/protoc-gen-go).

For the "github.com/grpc-ecosystem/grpc-gateway/examples/integration" you seem to have a mix of using the new "context" and the old "golang.org/x/net/context". I don't know how to resolve that.

https://gist.github.com/ensonic/1ddd754610de01ed99bb55ab52c7579b

FYI: I also get

FAIL	github.com/grpc-ecosystem/grpc-gateway/examples/integration [build failed]

without my change (using go version go1.10.2 linux/amd64). We will just use my fork for now, but I'd love to understand why your Go1.10.X builders would work and mine doesn't (also other PRs seem to have the same problem). Maybe consider adding some docs to make life for contributors easier.

johanbrandhorst · 2018-09-09T13:58:00Z

Hi @ensonic, we're made some changes to CI since you first had these problems, want to rebase and give it another go?

johanbrandhorst · 2018-10-17T11:10:21Z

Please rebase on master for new CI functionality.

This information is used by the default html doc generator and so it is nice to be able to provide it. Add the message to openapiv2.proto and extend template.go to handle it. Tested via: bazel build //examples/proto/examplepb:expamplepb_protoc_gen_swagger more bazel-bin/examples/proto/examplepb/a_bit_of_everything.swagger.json

ensonic · 2018-11-05T13:31:30Z

Still the same. For using it with bazel I can simply run: bazel test //... and it passes:

INFO: Elapsed time: 55.135s, Critical Path: 25.99s, Remote (0.00% of the time): [queue: 0.00%, setup: 0.00%, process: 0.00%]
INFO: 515 processes: 515 linux-sandbox.
INFO: Build completed successfully, 558 total actions
//codegenerator:go_default_xtest                                         PASSED in 0.2s
//examples/integration:go_default_xtest                                  PASSED in 0.6s
//protoc-gen-grpc-gateway/descriptor:go_default_test                     PASSED in 0.2s
//protoc-gen-grpc-gateway/gengateway:go_default_test                     PASSED in 0.2s
//protoc-gen-grpc-gateway/httprule:go_default_test                       PASSED in 0.2s
//protoc-gen-swagger:go_default_test                                     PASSED in 0.2s
//protoc-gen-swagger/genswagger:go_default_test                          PASSED in 0.2s
//runtime:go_default_test                                                PASSED in 0.2s
//runtime:go_default_xtest                                               PASSED in 0.2s
//utilities:go_default_xtest                                             PASSED in 0.2s

Executed 10 out of 10 tests: 10 tests pass.
INFO: Build completed successfully, 558 total actions

Running make test still fails for me with some error:

make test
mkdir -p _output
protoc -I /usr/bin//../include -I. --plugin=bin/protoc-gen-go --go_out=Mgoogle/protobuf/descriptor.proto=github.com/golang/protobuf/protoc-gen-go/descriptor,Mexamples/proto/sub/message.proto=github.com/grpc-ecosystem/grpc-gateway/examples/proto/sub,plugins=grpc:_output examples/proto/sub/message.proto
cp _output/github.com/grpc-ecosystem/grpc-gateway/examples/proto/sub/message.pb.go examples/proto/sub/message.pb.go || cp _output/examples/proto/sub/message.pb.go examples/proto/sub/message.pb.go
cp: cannot stat '_output/github.com/grpc-ecosystem/grpc-gateway/examples/proto/sub/message.pb.go': No such file or directory
mkdir -p _output
protoc -I /usr/bin//../include -I. --plugin=bin/protoc-gen-go --go_out=Mgoogle/protobuf/descriptor.proto=github.com/golang/protobuf/protoc-gen-go/descriptor,Mexamples/proto/sub/message.proto=github.com/grpc-ecosystem/grpc-gateway/examples/proto/sub,plugins=grpc:_output examples/proto/sub2/message.proto
cp _output/github.com/grpc-ecosystem/grpc-gateway/examples/proto/sub2/message.pb.go examples/proto/sub2/message.pb.go || cp _output/examples/proto/sub2/message.pb.go examples/proto/sub2/message.pb.go
protoc -I /usr/bin//../include -I. -Ithird_party/googleapis --plugin=bin/protoc-gen-go --go_out=Mgoogle/protobuf/descriptor.proto=github.com/golang/protobuf/protoc-gen-go/descriptor,Mexamples/proto/sub/message.proto=github.com/grpc-ecosystem/grpc-gateway/examples/proto/sub,plugins=grpc:. examples/proto/examplepb/echo_service.proto examples/proto/examplepb/a_bit_of_everything.proto examples/proto/examplepb/stream.proto examples/proto/examplepb/flow_combination.proto examples/proto/examplepb/wrappers.proto examples/proto/examplepb/unannotated_echo_service.proto examples/proto/examplepb/response_body_service.proto
go build -o bin/protoc-gen-grpc-gateway github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway
# github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway/descriptor
protoc-gen-grpc-gateway/descriptor/services.go:146:49: opts.ResponseBody undefined (type *annotations.HttpRule has no field or method ResponseBody)
Makefile:139: recipe for target 'bin/protoc-gen-grpc-gateway' failed

For the CI errors it looks like protoc-gen-swagger/options/openapiv2.pb.go needs to be re-generated, but your non-bazel build setup does not take care of it.

johanbrandhorst · 2018-11-05T14:32:07Z

Interesting, is that because of a dependency update or something? Would you be interested in submitting a PR to regenerate the file, or, regenerate it in this PR?

johanbrandhorst · 2018-11-05T14:36:40Z

So if you look at our CI tests it's actually saying that you still need to regenerate things. Bazel doesn't use the generated files in the same way (apparently) but we still need them. The License type needs to be added to the generated file.

johanbrandhorst · 2018-11-05T14:38:01Z

Don't run just make test locally, please regenerate the files as per the contribution guidelines: https://github.com/grpc-ecosystem/grpc-gateway/blob/master/CONTRIBUTING.md#i-want-to-regenerate-the-files-after-making-changes

ensonic · 2018-11-05T15:33:49Z

This looks better. The part about regenerating files should be one of the commands in 'how-to-contribute' and should be done before sending a PR :)

codecov-io · 2018-11-05T15:36:49Z

Codecov Report

Merging #644 into master will decrease coverage by 0.11%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #644      +/-   ##
==========================================
- Coverage   53.39%   53.27%   -0.12%     
==========================================
  Files          30       30              
  Lines        3362     3369       +7     
==========================================
  Hits         1795     1795              
- Misses       1392     1399       +7     
  Partials      175      175

Impacted Files	Coverage Δ
protoc-gen-swagger/genswagger/template.go	`38.39% <0%> (-0.3%)`	⬇️

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 dfdde99...6cc7699. Read the comment docs.

johanbrandhorst · 2018-11-05T15:45:35Z

Nice one, thanks!

ensonic · 2018-11-05T16:27:36Z

Sorry for being such a pain to get this in and thanks for merging!

johanbrandhorst · 2018-11-05T16:37:31Z

Thanks for the contribution :). Our old CI system was largely to blame, unfortunately there's still lots of good contributions like this laying around.

googlebot added the cla: yes label May 8, 2018

ensonic force-pushed the master branch from cfb5bf6 to 95a410f Compare May 8, 2018 08:11

achew22 reviewed May 8, 2018

View reviewed changes

ensonic force-pushed the master branch from 95a410f to 4e0ecd0 Compare May 22, 2018 14:32

johanbrandhorst added waiting on reporter enhancement openapi labels Sep 9, 2018

ensonic force-pushed the master branch from 4e0ecd0 to aad00fc Compare November 5, 2018 13:40

Regenerate checked-in files for the Info.License addition.

6cc7699

johanbrandhorst merged commit 844b78d into grpc-ecosystem:master Nov 5, 2018

ivucica mentioned this pull request Nov 6, 2018

go get -u github.com/grpc-ecosystem/grpc-gateway/protoc-gen-grpc-gateway fails on clean environment #731

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add 'License' message to the annotation proto. #644

Add 'License' message to the annotation proto. #644

ensonic commented May 8, 2018

ensonic commented May 8, 2018

ensonic commented May 8, 2018

ensonic commented May 8, 2018

achew22 May 8, 2018

ensonic May 8, 2018

ensonic commented May 11, 2018

ensonic commented May 16, 2018

achew22 commented May 17, 2018

ensonic commented May 17, 2018 •

edited

Loading

johanbrandhorst commented Sep 9, 2018

johanbrandhorst commented Oct 17, 2018

ensonic commented Nov 5, 2018 •

edited

Loading

johanbrandhorst commented Nov 5, 2018

johanbrandhorst commented Nov 5, 2018

johanbrandhorst commented Nov 5, 2018

ensonic commented Nov 5, 2018

codecov-io commented Nov 5, 2018

johanbrandhorst commented Nov 5, 2018

ensonic commented Nov 5, 2018

johanbrandhorst commented Nov 5, 2018

Add 'License' message to the annotation proto. #644

Add 'License' message to the annotation proto. #644

Conversation

ensonic commented May 8, 2018

ensonic commented May 8, 2018

ensonic commented May 8, 2018

ensonic commented May 8, 2018

achew22 May 8, 2018

Choose a reason for hiding this comment

ensonic May 8, 2018

Choose a reason for hiding this comment

ensonic commented May 11, 2018

ensonic commented May 16, 2018

achew22 commented May 17, 2018

ensonic commented May 17, 2018 • edited Loading

johanbrandhorst commented Sep 9, 2018

johanbrandhorst commented Oct 17, 2018

ensonic commented Nov 5, 2018 • edited Loading

johanbrandhorst commented Nov 5, 2018

johanbrandhorst commented Nov 5, 2018

johanbrandhorst commented Nov 5, 2018

ensonic commented Nov 5, 2018

codecov-io commented Nov 5, 2018

Codecov Report

johanbrandhorst commented Nov 5, 2018

ensonic commented Nov 5, 2018

johanbrandhorst commented Nov 5, 2018

ensonic commented May 17, 2018 •

edited

Loading

ensonic commented Nov 5, 2018 •

edited

Loading