-
Notifications
You must be signed in to change notification settings - Fork 68
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
Generate OpenAPI documentation #353
Conversation
b3100e4
to
bc1084a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Only minor note worth looking at is about filenames
@go install go.temporal.io/api/cmd/protogen@latest | ||
@go install google.golang.org/protobuf/cmd/protoc-gen-go@latest | ||
@go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@latest | ||
@go install github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-grpc-gateway@latest | ||
@go install github.com/pseudomuto/protoc-gen-doc/cmd/protoc-gen-doc@latest | ||
@go install github.com/grpc-ecosystem/grpc-gateway/v2/protoc-gen-openapiv2@latest | ||
@go install github.com/google/gnostic/cmd/protoc-gen-openapi@latest | ||
@go install github.com/mikefarah/yq/v4@latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not necessarily in this PR, but I wonder if we want to use fixed versions here (or in a go.mod somewhere). I fear a newer OpenAPI gen will make slight alterations to the output that will cause CI to fail for someone that isn't even working on these. But not blocking or anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I sure hope not. If we do we should discuss whether we want to pin the version of our protoc plugins as well. We already do for our server repo
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I could see some slight protoc-gen-openapi
improvement making some tiny inconsequential output change resulting in CI break. But I am not worried (we can cross that road when we get there), but yes I do think we've learned we prefer fixed versions even if we're not the most mindful of upgrading.
7b671c2
to
034ee87
Compare
What changed?
We now generate openapi v2 (swagger) and v3 specs as a part of proto compilation.
Why?
So that our customers can understand how to use our HTTP API
How did you test it?
Build and run temporalio/temporal#5393 (make start-dependencies + make start)