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 swagger spec and include auth APIs and common APIs #401

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

haoming29
Copy link
Contributor

Closes #352

This is a hand-coded Swagger specification of APIs in Pelican servers. For now, I included APIs for authentication and common APIs (/config, /health, etc.)

As a next step, we might want to programmatically generate this spec using annotations on handler functions, and there's existing Go package to handle spec generation for us: swaggo/swag

If you are using VSCode as your IDE, you can render the Swagger UI from this .yaml file by installing the OpenAPI (Swagger) Editor

@haoming29 haoming29 added the enhancement New feature or request label Nov 21, 2023
@haoming29 haoming29 added this to the v7.3.0 milestone Nov 21, 2023
@haoming29 haoming29 requested review from bbockelm and CannonLock and removed request for bbockelm November 21, 2023 21:12
@CannonLock
Copy link
Contributor

@haoming29 Is there a reason you picked yaml over json?

I am supposed to consume this in the frontend so it will be more convenient for me to have it as json.

@haoming29
Copy link
Contributor Author

haoming29 commented Nov 22, 2023

@haoming29 Is there a reason you picked yaml over json?

I am supposed to consume this in the frontend so it will be more convenient for me to have it as json.

@bbockelm suggested to use yaml to be more human-readable and I thought the swagger-ui npm package support .yaml by default? https://github.com/swagger-api/swagger-ui/blob/HEAD/docs/usage/configuration.md#configuration

@CannonLock
Copy link
Contributor

Okay, yaml works for now. When this is autogenerated we should do JSON as well as yaml.

The npm package only consumes json, its not a hard translation but I don't see any reason to make it if we can auto generate the file. -> https://www.npmjs.com/package/swagger-ui-react

@haoming29
Copy link
Contributor Author

Okay, yaml works for now. When this is autogenerated we should do JSON as well as yaml.

The npm package only consumes json, its not a hard translation but I don't see any reason to make it if we can auto generate the file. -> https://www.npmjs.com/package/swagger-ui-react

That makes sense to me

Copy link
Contributor

@CannonLock CannonLock left a comment

Choose a reason for hiding this comment

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

Approved.

Make sure to add another commit to your auth restructure so that these paths are maintained.

@haoming29 haoming29 merged commit 20c240e into PelicanPlatform:main Nov 22, 2023
6 checks passed
@haoming29 haoming29 deleted the swagger-spec branch November 22, 2023 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Swagger schema for Pelican
2 participants