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

docs(api): update api description #13429

Closed
wants to merge 7 commits into from

Conversation

walpox
Copy link
Contributor

@walpox walpox commented Jan 4, 2025

Proposed changes

  • Move the API description string to a separate variable to improve code readability.
  • Add content to the (Open)API description summarized from general sections of the REST API documentation.

Checklist

  • Lint and unit tests pass locally with my changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added documentation to describe my feature.
  • I have squashed my commits into logic units.
  • I have described the changes in the commit messages.

Other information

The rationale behind this change is to help the OpenAPI document stand on its own.

Relates to #12584

walpox added 2 commits January 4, 2025 23:13
Move the API description string to a separate variable to improve code readability.

Add content to the (Open)API description summarized from general sections of the REST API documentation.
weblate/api/spectacular.py Outdated Show resolved Hide resolved
Weblate's REST API is based on [Django REST framework](https://www.django-rest-framework.org).
You can interact with it on the `/api/` URL path by using the [Weblate Client]({get_doc_url(page='wlc')}) or any third-party REST client of your choice.

## Authentication
Copy link
Member

Choose a reason for hiding this comment

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

This is already covered in OpenAPI, it could be definitely improved, but better to have it in standard location than in text:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The OpenAPI spec allows to specify descriptions for the Security Scheme Object, so it should be possible to move information from the Authentication section in api_description to this other area of code.

I thought of moving this info in a separate PR. Do you prefer to do it in this one?

requests per day. On the other hand, authenticated requests are rate limited
to 5000 requests per hour by default.

## API rate limiting
Copy link
Member

Choose a reason for hiding this comment

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

This is already partly documented in response headers via add_middleware_headers:

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good remark. I have moved their description to the respective Python module.

weblate/api/spectacular.py Outdated Show resolved Hide resolved
Copy link
Member

@nijel nijel left a comment

Choose a reason for hiding this comment

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

Any thoughts on comments I've posted before? I'm still not sure what is the best approach on documenting these.

@walpox walpox marked this pull request as draft January 13, 2025 16:26
walpox and others added 2 commits January 13, 2025 19:10
Move information about the HTTP response headers related to rate limit
to variables representing those headers in the docs.py module.
@nijel
Copy link
Member

nijel commented Jan 16, 2025

This will need similar wrapping as introduced in #13546. Still, I'm not convinced that the description is the best place where to describe authentication and rate limiting.

@nijel
Copy link
Member

nijel commented Feb 6, 2025

The authorization is mostly covered by the standard features since 22b6a05, so let's close this one as most of its content has been incorporated elsewhere.

@nijel nijel closed this Feb 6, 2025
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