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

Add dev apis UI with swagger-ui #2991

Merged
merged 1 commit into from Jun 17, 2021
Merged

Conversation

antgamdia
Copy link
Contributor

@antgamdia antgamdia commented Jun 16, 2021

Description of the change

This PR brings an easy way to test the API changes just using the generated OpenAPI v2 document. For doing so, this PR is embedding and serving a tuned up swagger-ui index.html.

apisUI.mp4

Benefits

Make developers happier :)

Possible drawbacks

N/A

Applicable issues

Additional information

Ideally, it (the makefile perhaps) should retrieve the latest swagger-ui version, perform the replacements, etc. But, given that it is a transient change, we shouldn't invest much time here, IMHO.


// TODO(agamez): remove these '/openapi.json' and '/docs' paths. They are serving a
// static 'swagger-ui' dashboard with hardcoded values just intended for develoment purposes.
// This docs will eventually converge into the docs already (properly) served by the dashboard
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear to me how this openapi.json has hard-coded values... isn't it the same openapi.json that gets server by the frontend? More generally, why don't we just serve it like this permanently so it works both in dev as well as when deployed on the cluster? (I think a go binary is pretty efficient at serving a static file, but check).

Or is the issue rather the index.html (haven't checked the content, too large for the we UI so I assume it's just static webui for openapi)? If so, couldn't we still leave this permanently, but have the frontend handle the same path with the proper docs? So in devel, you get this one by default, but when configured on the cluster, this handler (for /docs) is never executed because the frontend already handles the path or similar?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your comments! Summarizing what we discussed offline:

  • The openapi.json is not an additional file but simply the path in which the usual file (the one generated with buf generate) is served. There is no problem in continue using this path in the future. In fact, it even makes more sense (since this component is responsible for its own docs, more cohesion)

  • The issue is the index.html: I wasn't able to simply use FileServer from gwmux.Handle function (which takes a *runtime.HandlerFunc) without refactoring a little bit the code (see https://github.com/lstoll/grpc-rest-example/blob/master/go/server/main.go#L63)

So, IMHO this is a reasonable change considering the scope of this PR (just a temporary UI for playing around while this comp is under heavy development)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging since the checks passed but wasn't properly notified back to github

@antgamdia antgamdia merged commit 0da5e41 into vmware-tanzu:master Jun 17, 2021
@antgamdia antgamdia deleted the addApisUI branch June 17, 2021 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add swaggerui in kubeapps-apis
2 participants