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

Improve API documentation #636

Merged
merged 7 commits into from
Nov 27, 2024
Merged

Improve API documentation #636

merged 7 commits into from
Nov 27, 2024

Conversation

dgruano
Copy link
Contributor

@dgruano dgruano commented Nov 13, 2024

Hi!

First of all, thank you for putting this incredible resource out there!

I am new to RNAcentral and had some trouble navigating the API to see its capabilities. Since it is implemented using Django REST framework, I was certain that some automatic documentation could be done. So here's what I did:

  • Implement drf_spectacular, a tool for OpenAPI schema generation.
  • Add /schema endpoint to return OpenAPI standard schema, so that it can be used in other applications (for me, it was Postman)
  • Include endpoints with UIs to navigate and test the API. These are /schema/redoc and schema/swagger-ui. For instance, swagger-ui looks like this:
    image

Happy to hear your comments and to edit the /api page with the new information if you decide this is worth merging.

- Implement drf_spectacular
- Add /schema endpoint to build OpenAPI standard
- Include UIs to navigate and test the API
@carlosribas
Copy link
Contributor

Hi @dgruano,

Thank you so much for your contribution, and I'm glad you find RNAcentral useful! I appreciate the effort you put into implementing drf_spectacular and adding the schema and UI endpoints - this is a great idea that can definitely improve the API's usability. I actually use drf_spectacular in another project, so I'm familiar with its benefits.

I won't be able to fully review and test this until next week, but I wanted to let you know that I'll be looking at it in detail soon. Thanks again for your contribution! I'll be in touch.

@dgruano
Copy link
Contributor Author

dgruano commented Nov 13, 2024

Hi @carlosribas,

Great to hear back from you so promptly! If it makes the review process any easier, here are some key points imo:

  • I think there's no need to keep both swagger and redoc interfaces.
  • The URL for the swagger/redoc documentations could be changed.
  • The schema could be customized (for instance, to group "v1" and "current" endpoints and avoid redundancy).

Looking forward to hearing your comments!

@carlosribas
Copy link
Contributor

Hi @dgruano,

Thank you once more for your contribution to our code! I have reviewed the Pull Request and made a few changes, as I believe we should:

  • Keep the API functioning in the same way.
  • Add a new link to the Swagger page.

As you may have noticed, drf_spectacular generates several warnings about improvements we need to make to display accurate information. While this doesn’t prevent us from using it, fixing these issues will take time. I need to assess whether it’s worth addressing them now, considering we have plans to modernize our website in the near future.

I still need to run a few more tests, but I want to let you know that your idea is great, and we plan to include it on our site soon. Feel free to share more suggestions anytime! Thank you.

@dgruano
Copy link
Contributor Author

dgruano commented Nov 25, 2024

Thanks for giving it a deeper look, and happy to see it was helpful! I agree there's no point in diving too deep into improving the API at its current state if a major refactoring is planned. This already helps the novice user to quickly understand its capabilities.

I 100% agree with the changes you made. Looking forward to seeing them in production!

If I may add another suggestion, maybe adding further explanation on how to run a search job through the API could be nice to programatically access results using parameters that are not immediately supported through the API.

@carlosribas carlosribas merged commit 60a76f5 into RNAcentral:master Nov 27, 2024
1 of 3 checks passed
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.

2 participants