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

Switch to OpenAPI v2 (instead of swagger) to fix API page generation. #2135

Open
4 tasks
SpicyLemon opened this issue Aug 23, 2024 · 1 comment
Open
4 tasks
Labels
bug Something isn't working documentation Improvements or additions to documentation proto Protobuf files work
Milestone

Comments

@SpicyLemon
Copy link
Contributor

Summary of Bug

The make proto-gen and make proto-swagger-gen targets currently fail.

When using the ghcr.io/cosmos/proto-builder:0.14.0 docker image, go-related stuff fails because our go.mod says to use go v1.23, but that image doesn't have that version of go. So make proto-gen fails with this: go: download go1.23 for linux/arm64: toolchain not available.

If we switch it to 0.15.0 then make proto-gen succeeds, but make proto-swagger-gen fails with Failure: plugin swagger: could not find protoc plugin for name swagger - please make sure protoc-gen-swagger is installed and present on your $PATH. That's because SDK has switched their proto/buf.gen.swagger.yaml to use openapiv2, but we still use swagger (in the name field). So when they built the ghcr.io/cosmos/proto-builder:0.15.0 image, they didn't include the protoc-gen-swagger utility, since they don't need it anymore.

Version

main at c70232484 Bump go to v1.23 (from v1.21) (#2132)

Steps to Reproduce

Run either make proto-gen or make proto-swagger-gen.

To Fix

  • Bump the proto image version to 0.15.0 (or later).
  • Update proto/buf.gen.swagger.yaml to use openapiv2
  • Review our proto related make targets and scripts, comparing them to what the SDK has and make any relevant updates. E.g. I think we might be able to take the if statements out of the make targets now.

Also, the SDK does not include Tx endpoints in their swagger stuff. We should consider either A) adding them to our stuff or B) removing ours or C) leaving it as it is. I'm pretty sure none of the Tx endpoints actually work from the swagger page, but it's nice to have them there for the documentation. To add them, we'd probably have to generate all of the SDK's stuff on our own (instead of just copying their swagger.yaml), but we (hopefully) have all the protos in the third_party directory, so it might not be too hard. Option A is nice because then there's a more complete picture of available functionality (and we could probably have them separated by module like ours), but requires extra work. Option B is nice because it matches what the SDK does and doesn't cause as much confusion when someone tries one of our Tx endpoints from that page, and it fails, and is a pretty simple change, but ends up hiding functionality. Option C is nice because it's the easiest, but still hides some functionality.

As of right now, I'd like to see us do option A, and C is okay, but I'm not a fan of B. It's nice to present a more complete picture of functionality even if you can't use the swagger page to try out the Tx endpoints.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@SpicyLemon SpicyLemon added this to the v1.20.0 milestone Aug 23, 2024
@SpicyLemon SpicyLemon added bug Something isn't working documentation Improvements or additions to documentation proto Protobuf files work labels Aug 23, 2024
@iramiller
Copy link
Member

Having a complete set of endpoints is nice since this information can be combined with ReDoc to make very readable documentation such as: https://test.provlabs.io/vault/docs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation proto Protobuf files work
Projects
Development

No branches or pull requests

2 participants