Deduplicate HTTP route index entries across netapi doc pages (#69724)#69725
Open
ggiesen wants to merge 3 commits into
Open
Deduplicate HTTP route index entries across netapi doc pages (#69724)#69725ggiesen wants to merge 3 commits into
ggiesen wants to merge 3 commits into
Conversation
The rest_cherrypy, rest_tornado and rest_wsgi doc pages document
overlapping HTTP routes, so each shared route registered multiple
httpdomain index entries. sphinxcontrib-httpdomain only detects the
duplicates in merge_domaindata, which runs when sphinx -j parallel
reader chunks are merged, so the -W builds in tools docs (Prepare
Release, Documentation) fail intermittently depending on where the
chunk boundary lands:
WARNING: duplicate HTTP post method definition / in
doc/ref/netapi/all/salt.netapi.rest_tornado.rst, other instance
is in doc/ref/netapi/all/salt.netapi.rest_wsgi.rst
When it hits, the release patch artifact is never produced and every
downstream build job in the run fails with 'Artifact not found'.
Mark the tornado and wsgi copies of the shared routes with :noindex:
so each route is registered exactly once, by the canonical
rest_cherrypy reference. noindex'd directives never enter the domain
data, so the merge collision is impossible under any chunking. Page
content is unchanged and no :http: cross-references exist that could
be affected. This also makes the HTTP routing index deterministic; it
previously pointed at whichever page the readers processed last.
Fixes saltstack#69724
a15e6b4 to
8c0c37c
Compare
1 task
sphinxcontrib-httpdomain's add_target_and_index always appends the signature anchor and only gates the global route registration behind :noindex:, but Sphinx's ObjectDescription.run skips the whole method when noindex is set, so the previous commit's dedup also dropped the per-page anchors and permalinks from the rest_tornado and rest_wsgi endpoint signatures. Anchors are per-document HTML ids and cannot collide across pages; only the global registration can produce the parallel-merge duplicate warnings. Add a small extension that hides the noindex option from Sphinx's outer gate and re-presents it to httpdomain's inner gate, restoring the anchors and permalinks (existing deep links into those pages keep working) while the routes stay out of the domain data, so the duplicate-route collision remains impossible under any chunking.
Contributor
Author
|
The behaviour the doc/_ext/salthttpanchors.py extension works around has been proposed as a fix upstream (sphinx-contrib/httpdomain#112, issue sphinx-contrib/httpdomain#111). If that lands and the docs.lock pin is eventually bumped past it, the extension can simply be deleted; nothing else depends on it. |
2 tasks
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
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.
What does this PR do?
Makes the
-Wdocs builds deterministic by removing duplicate HTTP route registrations across the netapi doc pages. Therest_cherrypy,rest_tornadoandrest_wsgipages document overlapping routes (9 shared between cherrypy and tornado, plusPOST /also in wsgi); each shared route therefore registered several httpdomain index entries. sphinxcontrib-httpdomain (2.0.0 as pinned) only detects duplicates inmerge_domaindata, which runs whensphinx -jparallel reader chunks are merged, sotools docs man/html(SPHINXOPTS=-W -j auto --keep-going) fail intermittently depending on where the chunk boundary happens to land:When it hits in Prepare Release, the release patch artifact is never produced and every downstream job in the run (Documentation, Source Tarball, all package/onedir builds) dies with "Artifact not found" -- one flaky warning takes out the whole run.
This PR adds
:noindex:to the tornado and wsgi copies of the shared routes, keepingrest_cherrypyas the single indexed, canonical instance of each.:noindex:directives never enter the domain data, so the merge collision becomes impossible under any chunking or worker count. As a side benefit the HTTP routing index becomes deterministic; previously shared routes pointed at whichever page the readers processed last.What issues does this PR fix or reference?
Fixes #69724
Previous Behavior
-W -j autodocs builds failed intermittently with duplicate HTTP method definition warnings, cascading into full-run artifact failures.New Behavior
Every HTTP route registers exactly once. Verified with the pinned docs.lock (sphinx 7.0.1, httpdomain 2.0.0): the pickled build environment previously showed shared routes registered by
rest_tornado/rest_wsgi; with this change every netapi route is registered once, byrest_cherrypy.make manandmake htmlwith-W -j auto --keep-goingbuild clean (repeated across several-jvalues); the tornado and wsgi pages still render their full endpoint documentation, and there are no:http:<method>:cross-references in the tree that could be affected.Merge requirements satisfied?
Docs-only change; no runtime code paths affected, so no unit tests apply.
Commits signed with GPG?
No
Notes from validation
:noindex:lines are indented to the directive content column (not the option column). With the shallower indent, docutils dedents the block by the option line and the directive content nested-parses as a block quote, which silently disables the doc-field transformation (Status Codes/Request Headersgrouping and links) on those endpoints while-Wstays green. Rendering was verified byte-equivalent to the pre-change pages with the content-column indent.:noindex:alone would also drop the HTML anchors/permalinks on the tornado/wsgi signatures, because Sphinx skipsadd_target_and_indexentirely undernoindex-- even though httpdomain itself only gates the global registration behind the option and always appends the anchor. Since anchors are per-page HTML ids that cannot collide across pages (only the global domain data can produce the parallel-merge warning), the second commit adds a smalldoc/_ext/salthttpanchors.pyextension that hides the option from Sphinx's outer gate and re-presents it to httpdomain's inner gate: the anchors and permalinks stay (existing deep links into those pages keep working), while the routes stay out of the domain data. Verified in the built HTML: all 10 tornado anchors (#post--login,#get--events, ...) and 28 permalinks present, domain data still registered solely by the cherrypy page.-Wwith-j auto,-j 2, and-j 4.