Maintenance: Remove sphinx-build-compatibility
#595
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
amotl
force-pushed
the
collab/no-sphinx-build-compatibility
branch
from
38f869a to
a0076fa
Compare
amotl
commented
Apr 7, 2025
Comment on lines
-133
to
-151
| # Add project information to the template context. | ||
| context = { | ||
| 'html_theme': config.html_theme, | ||
| 'current_version': os.environ.get("READTHEDOCS_VERSION_NAME"), | ||
| 'version_slug': version_slug, | ||
|
|
||
| # NOTE: these are used to dump them in some JS files and to build the URLs in flyout. | ||
| # However, we are replacing them with the new Addons. | ||
| # I wouldn't include them in the first version of the extension. | ||
| # We could hardcode them if we want, tho. | ||
| # | ||
| # 'MEDIA_URL': "{{ settings.MEDIA_URL }}", | ||
| # 'STATIC_URL': "{{ settings.STATIC_URL }}", | ||
| # 'proxied_static_path': "{{ proxied_static_path }}", | ||
|
|
||
| 'PRODUCTION_DOMAIN': production_domain, | ||
| 'versions': versions, | ||
| "downloads": downloads, | ||
| "subprojects": subprojects, | ||
|
|
||
| 'slug': project_slug, | ||
| 'rtd_language': os.environ.get("READTHEDOCS_LANGUAGE"), | ||
| 'canonical_url': os.environ.get("READTHEDOCS_CANONICAL_URL"), | ||
|
|
||
| # NOTE: these seem to not be used. | ||
| # 'name': u'{{ project.name }}', | ||
| # 'analytics_code': '{{ project.analytics_code }}', | ||
| # 'single_version': {{ project.is_single_version }}, | ||
| # 'programming_language': u'{{ project.programming_language }}', | ||
|
|
||
| 'conf_py_path': conf_py_path, | ||
| # Used only for "readthedocs-sphinx-ext" which we are not installing anymore. | ||
| # 'api_host': '{{ api_host }}', | ||
| # 'proxied_api_host': '{{ project.proxied_api_host }}', | ||
|
|
||
| 'github_user': github_user, | ||
| 'github_repo': github_repo, | ||
| 'github_version': os.environ.get("READTHEDOCS_GIT_IDENTIFIER"), | ||
| 'display_github': github_user is not None, | ||
| 'bitbucket_user': bitbucket_user, | ||
| 'bitbucket_repo': bitbucket_repo, | ||
| 'bitbucket_version': os.environ.get("READTHEDOCS_GIT_IDENTIFIER"), | ||
| 'display_bitbucket': bitbucket_user is not None, | ||
| 'gitlab_user': gitlab_user, | ||
| 'gitlab_repo': gitlab_repo, | ||
| 'gitlab_version': os.environ.get("READTHEDOCS_GIT_IDENTIFIER"), | ||
| 'display_gitlab': gitlab_user is not None, | ||
| 'READTHEDOCS': True, | ||
| 'using_theme': (config.html_theme == "default"), | ||
| 'new_theme': (config.html_theme == "sphinx_rtd_theme"), | ||
| 'source_suffix': ".rst", | ||
| 'ad_free': False, | ||
| 'docsearch_disabled': False, | ||
|
|
||
| # We don't support Google analytics anymore. | ||
| # See https://github.com/readthedocs/readthedocs.org/issues/9530 | ||
| 'user_analytics_code': "", | ||
| 'global_analytics_code': None, | ||
|
|
||
| 'commit': os.environ.get("READTHEDOCS_GIT_COMMIT_HASH")[:8], | ||
| } |
There was a problem hiding this comment.
We'd need to check this whole bunch, if we also set all context variables accordingly?
How would you derive any actions from this? Let's discuss?
Comment on lines
-195
to
-158
| # For sphinx >=1.8 we can use html_baseurl to set the canonical URL. | ||
| # https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-html_baseurl | ||
| if version_info >= (1, 8): | ||
| if not hasattr(config, 'html_baseurl'): | ||
| config.html_baseurl = context['canonical_url'] | ||
| context['canonical_url'] = None |
There was a problem hiding this comment.
I can't discover a reference to canonical_url in our code, just about custom_baseurl. That's probably a gap we need to fill?
Comment on lines
-203
to
-166
| if hasattr(config, 'html_context'): | ||
| for key in context: | ||
| if key not in config.html_context: | ||
| config.html_context[key] = context[key] | ||
| else: | ||
| config.html_context = context |
There was a problem hiding this comment.
As you can see here and there, we applied trial-and-error techniques in the past to make things work without knowing too much about relevant details.
Maybe this is the key element we have been missing before: First, populate the template context. Then, propagate all ingredients into the html_context 1:1, in a generic way. I think we may want to carry this gem over to our crate/theme/rtd/conf/__init__.py as well.
Comment on lines
-242
to
-203
| # Make sure our build directory is always excluded | ||
| if not hasattr(config, "exclude_patterns"): | ||
| config.exclude_patterns = [] | ||
| config.exclude_patterns.extend(['_build']) |
There was a problem hiding this comment.
That also looks interesting. We can add it directly to our theme's configuration.
Comment on lines
56
to
131
| # We are using APIv2 to pull active versions, downloads and subprojects | ||
| # because APIv3 requires a token. | ||
| try: | ||
| response_project = requests.get( | ||
| f"{scheme}://{production_domain}/api/v3/projects/{project_slug}/", | ||
| timeout=2, | ||
| ).json() | ||
| language = response_project["language"]["code"] | ||
| except Exception: | ||
| logger.warning( | ||
| "An error ocurred when hitting API to fetch project language. Defaulting to 'en'.", | ||
| exc_info=True, | ||
| ) | ||
| language = "en" | ||
|
|
||
| try: | ||
| response_versions = requests.get( | ||
| f"{scheme}://{production_domain}/api/v3/projects/{project_slug}/versions/?active=true", | ||
| timeout=2, | ||
| ).json() | ||
| versions = [ | ||
| (version["slug"], f"/{language}/{version['slug']}/") | ||
| for version in response_versions["results"] | ||
| ] | ||
| except Exception: | ||
| logger.warning( | ||
| "An error ocurred when hitting API to fetch active versions. Defaulting to an empty list.", | ||
| exc_info=True, | ||
| ) | ||
| versions = [] | ||
|
|
||
| try: | ||
| downloads = [] | ||
| for version in response_versions["results"]: | ||
| if version["slug"] != version_slug: | ||
| continue | ||
|
|
||
| for key, value in version["downloads"]: | ||
| downloads.append( | ||
| ( | ||
| key, | ||
| value, | ||
| ), | ||
| ) | ||
| except Exception: | ||
| logger.warning( | ||
| "An error ocurred when generating the list of downloads. Defaulting to an empty list.", | ||
| exc_info=True, | ||
| ) | ||
| downloads = [] | ||
|
|
||
| try: | ||
| subprojects = [] | ||
| response_project = requests.get( | ||
| f"{scheme}://{production_domain}/api/v2/project/?slug={project_slug}", | ||
| timeout=2, | ||
| ).json() | ||
| project_id = response_project["results"][0]["id"] | ||
|
|
||
| response_subprojects = requests.get( | ||
| f"{scheme}://readthedocs.org/api/v2/project/{project_id}/subprojects/", | ||
| timeout=2, | ||
| ).json() | ||
| for subproject in response_subprojects["subprojects"]: | ||
| subprojects.append( | ||
| ( | ||
| subproject["slug"], | ||
| subproject["canonical_url"], | ||
| ), | ||
| ) | ||
| except Exception: | ||
| logger.warning( | ||
| "An error ocurred when hitting API to fetch project/subprojects. Defaulting to an empty list.", | ||
| exc_info=True, | ||
| ) | ||
| subprojects = [] |
There was a problem hiding this comment.
This is the section which has most impact on runtime, because it fires loads of additional requests per rebuild to the RTD API.
Maybe we can start by just getting rid of this section? In this spirit, we wouldn't need to tackle "removing the whole layer at once", but instead are able to progress iteratively. wdyt?
amotl
commented
Apr 7, 2025
Comment on lines
-56
to
-58
| # We are using APIv2 to pull active versions, downloads and subprojects | ||
| # because APIv3 requires a token. |
There was a problem hiding this comment.
Maybe it is better to use APIv3 again, and supply a token, instead of triggering rate limits?
Comment on lines
58
to
69
| try: | ||
| response_project = requests.get( | ||
| f"{scheme}://{production_domain}/api/v3/projects/{project_slug}/", | ||
| timeout=2, | ||
| ).json() | ||
| language = response_project["language"]["code"] | ||
| except Exception: | ||
| logger.warning( | ||
| "An error ocurred when hitting API to fetch project language. Defaulting to 'en'.", | ||
| exc_info=True, | ||
| ) | ||
| language = "en" |
There was a problem hiding this comment.
We don't need to ask the API about the language. We can just set it to en.
Comment on lines
71
to
85
| try: | ||
| response_versions = requests.get( | ||
| f"{scheme}://{production_domain}/api/v3/projects/{project_slug}/versions/?active=true", | ||
| timeout=2, | ||
| ).json() | ||
| versions = [ | ||
| (version["slug"], f"/{language}/{version['slug']}/") | ||
| for version in response_versions["results"] | ||
| ] | ||
| except Exception: | ||
| logger.warning( | ||
| "An error ocurred when hitting API to fetch active versions. Defaulting to an empty list.", | ||
| exc_info=True, | ||
| ) | ||
| versions = [] |
There was a problem hiding this comment.
We certainly need to know about project versions, so we need to keep this?
Comment on lines
87
to
105
| try: | ||
| downloads = [] | ||
| for version in response_versions["results"]: | ||
| if version["slug"] != version_slug: | ||
| continue | ||
|
|
||
| for key, value in version["downloads"]: | ||
| downloads.append( | ||
| ( | ||
| key, | ||
| value, | ||
| ), | ||
| ) | ||
| except Exception: | ||
| logger.warning( | ||
| "An error ocurred when generating the list of downloads. Defaulting to an empty list.", | ||
| exc_info=True, | ||
| ) | ||
| downloads = [] |
There was a problem hiding this comment.
What are "downloads"? It feels like we don't need it?
Comment on lines
107
to
82
| try: | ||
| subprojects = [] | ||
| response_project = requests.get( | ||
| f"{scheme}://{production_domain}/api/v2/project/?slug={project_slug}", | ||
| timeout=2, | ||
| ).json() | ||
| project_id = response_project["results"][0]["id"] | ||
|
|
||
| response_subprojects = requests.get( | ||
| f"{scheme}://readthedocs.org/api/v2/project/{project_id}/subprojects/", | ||
| timeout=2, | ||
| ).json() | ||
| for subproject in response_subprojects["subprojects"]: | ||
| subprojects.append( | ||
| ( | ||
| subproject["slug"], | ||
| subproject["canonical_url"], | ||
| ), | ||
| ) |
There was a problem hiding this comment.
We are currently not using subprojects, so we can think about removing this snippet, in the same spirit to fire less requests to the RTD API?
amotl
force-pushed
the
collab/no-sphinx-build-compatibility
branch
from
a0076fa to
68b2052
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
About
This patch makes a humble start in resolving a problem which started to cause more pains recently. 1
Progress
@msbt: We can work together on this one / you could possibly take over from here, optionally accompanied by orientation flights we will have together?
Footnotes
We know about it, because the rate of failing nightly rebuilds significantly started increasing last week, while we started using Rapporto the other day to actually get informed about those. ↩