Skip to content

Refactor: remove Version.documentation_type #10042

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

humitos
Copy link
Member

@humitos humitos commented Feb 16, 2023

We are planning to support any doctool and we want to get rid of the documentation_type attribute and make our logic more generic to work on most scenarios without specific logic per doctool.

We are planning to suppose _any_ doctool and we want to get rid of the
`documentation_type` attribute and make our logic more generic to work on most
scenarios without specific logic per doctool.
@humitos
Copy link
Member Author

humitos commented Jun 26, 2023

Summarizing the required changes here:

  • EmbedAPIv1 needs to be removed since it uses documentation_type to decide how to parse the file. It seems that /api/v1/embed/ and /api/v2/embed hit the same code. I took a look at CF and there are some users already using v2: https://dash.cloudflare.com/76f653d51b8df4e9e956eb83a5ff42f4/readthedocs.org/analytics
    /traffic?path~starts=%2Fapi%2Fv2%2Fembed%2F&time-window=43200. We will need a plan to deprecate it.
  • Footer API is using it to generate a path and remove the /index part of the URL when it's Sphinx HTMLDIR or MKDOCS. It may makes sense to review this logic because I'm not 100% sure how it works and why it's needed. Also, this endpoint will be replaced by the new Addons flyout and its /_/readthedocs-config/ API endpoint.
  • The same logic about striping the index.html is used in the PageSearchSerializer APIv2 serializer.
  • Telemetry data collector use this type to decide whether or not collecting Sphinx specific data. This code could easily be adapted by checking for the existence telemetry.json file instead.
  • We require Search: Only use generic parsers #10272 to always use the GenericParser to create search indexes.

It seems the only blocker here is "understanding the logic behind the stripping of index.html from FooterAPI and PageSearchSerializer. Then, we can re-write that logic to not be based on the doctype or remove it completely if it's not required.

I think we are close to be able to move forward here.

humitos added a commit that referenced this pull request Jun 30, 2023
This API is deprecated and nobody is currently using it.
I checked this on Cloudflare and I didn't find people hitting this endpoint at all.

Related #10042
@humitos
Copy link
Member Author

humitos commented Jul 11, 2024

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.

Remove project.documentation_type from all our serving code, and models
1 participant