grpc-gateway
grpc-gateway copied to clipboard
grpc-ecosystem
Update or remove swagger-codegen
We're on 2.1.6 while 2.2.0 is latest: https://github.com/swagger-api/swagger-codegen/releases
As it stands this creates a barrier to contribution as folks will run into errors.
@tmc, Do we need to do more to upgrade than change the hardcoded compatibility string? I didn't see anything in there listed as a breaking change
@achew22 afaict it dumps out code with different package names into the same directory.
I pushed https://travis-ci.org/tmc/grpc-gateway/builds/177507616 to demonstrate
with 2.2.0 or 2.2.1 I get:
go test -race github.com/grpc-ecosystem/grpc-gateway/...
can't load package: package github.com/grpc-ecosystem/grpc-gateway/examples/clients: found packages abe (a_bit_of_everything_nested.go) and echo (echo_service_api.go) in /Users/tmc/go/src/github.com/grpc-ecosystem/grpc-gateway/examples/clients
make: *** [test] Error 1``
Thoughts on removing or disabling the swagger-codegen calls? It's a bit tangential and it will be a contribution barrier.
looking more closely this appears to be the --additional-properties packageName=echo parameter to swagger-codegen not affecting the output directory anymore.
It feels like a test that is testing something outside the control of this project. It does, however, add a nice e2e test to the repo. Honestly I don't even remember this commit going by which explains my total lack of understanding in my earlier comment.
Let's ask ourselves the question "What is this testing and can it be tested in another way" specifically without bringing in a dependency on swagger-codegen
If were were actually exercising the swagger-codegen-generated code in test I'd be more inclined to keep it around. Right now it's basically testing well-formedness of the generated swagger output.
the current version of swagger-codegen (that you get with brew install for example) is 2.3.1 - CONTRIBUTING.md says "use swagger-codegen 2.2.2, not newer versions". @johanbrandhorst - would it be possible to make grpc-gateway to work with 2.3.1? Thanks.
With the new CI system proposed in #772 this will be trivial to accomplish, and I'm not sure what if anything stands in our way to do it, but lets get back to this once #772 has been merged.
@tmc @achew22 I think we could probably remove it altogether as well - personally when I'm generating swagger files I don't use swagger-codegen as the Go code is next to useless anyway. Again, this can easily be accomplished after #772 has been merged. I don't think it adds any extra confidence as it is.
Yes :). I don't think it adds anything. Would you like to create a new PR with it removed? We can remove it from the Dockerfile as well.
Ok after some discussion a requirement has presented itself; if we remove swagger-codegen, we must replace it with some other method by which we can validate that our output swagger files are sensible. I have some personal experience with using https://github.com/go-swagger/go-swagger, which seems to do the job. If anyone wants to replace swagger-codegen we should consider it at the same time.
As mentioned in https://github.com/grpc-ecosystem/grpc-gateway/pull/795#issuecomment-435618237, the use of swagger-codegen is now leading to slightly annoying travis files appearing after every generation. We can fix this with a simple .gitignore rule, but it's just another thing we shouldn't have to do.