-
Notifications
You must be signed in to change notification settings - Fork 200
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
build: generate OpenAPI from grpc #3357
Conversation
9c38ac4
to
4c9ac6e
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3357 +/- ##
==========================================
+ Coverage 64.41% 64.44% +0.02%
==========================================
Files 172 172
Lines 13820 13820
==========================================
+ Hits 8902 8906 +4
+ Misses 4234 4232 -2
+ Partials 684 682 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Looks great! just replying to some of your questions in the PR description:
I think at the root of the project? We could setup an action maybe to copy it over to https://github.com/flipt-io/flipt-openapi which drives the updates in our docs, or maybe just remove It seems that Mintlify can pull the openapi spec from anywhere, so maybe we can give it a link to the raw file on GitHub at the root?
No. We don't want to show the legacy evaluation endpoints.
Yes. I think we should add this as part of the OpenAPI docs |
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 think this is great @erka 🙌 Awesome catch. Yeah, I think as Mark suggested, maybe get it into the root of the project? Then lets get it in 💪
@markphelps could I rename |
@erka how do you mean? do you mean for OFREP? or something else? |
I played a bit with other side of OpenApi generation and I would like to make the api a bit nicer there. For example, we have such operation_ids currently /api/v1/namespaces:
get:
tags:
- Flipt
operationId: NamespacesService.list
--- omitted ---
/ofrep/v1/evaluate/flags/{key}:
post:
tags:
- OFREPService
operationId: ofrep.evaluate After code generation, I get the client with the methods func main() {
client, _ := gen.NewClientWithResponses("http://localhost:8080")
client.NamespacesServiceList(context.TODO(), ...)
client.OfrepEvaluate(context.TODO(), "flag", ...)
} It isn't very clear for me what @markphelps wdyt? |
I think that's fine, although I would suggest |
Signed-off-by: Roman Dmytrenko <rdmytrenko@gmail.com>
Signed-off-by: Roman Dmytrenko <rdmytrenko@gmail.com>
replace flipt:sdk:ignore with api_visibility restriction. update operation ids and tags Signed-off-by: Roman Dmytrenko <rdmytrenko@gmail.com>
d4da563
to
de99bd4
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! thank you @erka !!
Use google/gnostic to generate OpenAPI v3 schema base on the proto files.
This PR has mixed
option (google.api.http)
andgrpc_api_configuration=rpc/flipt/flipt.yaml
together. If the path should be included into OpenAPI spec it should be usegoogle.api.http
for path configurationIssues:
openapi.yaml
github.com/google/gnostic/openapiv3
withgit.luolix.top/google/gnostic-models/openapiv3
legacy evaluation endpoints are missing. Is it a problem?related flipt-io/flipt-openapi/issues/25