kaxil commented on code in PR #73477:
URL: https://github.com/apache/airflow/pull/73477#discussion_r4070126458
##########
devel-common/src/docs/build_docs.py:
##########
@@ -693,52 +729,19 @@ def build_docs(
all_spelling_errors.update(package_spelling_errors)
if not one_pass_only:
- # Build documentation for some packages again if it can help them.
- package_build_errors = retry_building_docs_if_needed(
- all_build_errors=all_build_errors,
- all_spelling_errors=all_spelling_errors,
- autobuild=autobuild,
- docs_only=docs_only,
- jobs=jobs,
- verbose=verbose,
- package_build_errors=package_build_errors,
- originally_built_packages=packages_to_build,
- # If spellchecking fails, we need to rebuild all packages first in
case some references
- # are broken between packages
- rebuild_all_packages=spellcheck_only,
Review Comment:
Yes. The inventory is written whenever the build runs to completion,
whatever it failed on, because under `-W` Sphinx (8.1 in the CI image, same
code in 9.x) counts warnings and sets the exit status at the end instead of
aborting on the first one. Only an actual exception part-way through skips it,
since the environment may not hold every document then.
Tried what you suggested, same ftp/sftp setup (new label in ftp, referenced
from sftp), no misspelling this time:
Case: title underline too short plus unexpected indentation in ftp:
```
apache-airflow-providers-ftp : Finished spell-checking with errors
providers/ftp/docs/index.rst:167: WARNING: Title underline too short.
build finished with problems, 2 warnings (with warnings treated as
errors).
apache-airflow-providers-sftp : Finished spell-checking with errors
providers/sftp/docs/index.rst:157: WARNING: undefined label:
'apache-airflow-providers-ftp:ftp-brand-new-label'
Packages to rebuild: ['apache-airflow-providers-sftp']
apache-airflow-providers-sftp : Finished spell-checking successfully
No packages to retry. No more passes are needed.
ftp inventory present: yes
```
Case: the above plus an unknown directive in ftp (an ERROR-level docutils
message) plus a misspelling:
```
apache-airflow-providers-ftp : Finished spell-checking with errors
providers/ftp/docs/index.rst:167: WARNING: Title underline too short.
providers/ftp/docs/index.rst:172: ERROR: Unknown directive type
"thisdirectivedoesnotexist".
WARNING: Found 1 misspelled words
build finished with problems, 4 warnings (with warnings treated as
errors).
apache-airflow-providers-sftp : Finished spell-checking with errors <-
undefined label, first pass
Packages to rebuild: ['apache-airflow-providers-sftp']
apache-airflow-providers-sftp : Finished spell-checking successfully
No packages to retry. No more passes are needed.
ftp inventory present: yes
```
In both, the run exits 1 and the summary shows only the genuine ftp errors;
the "missing label" in sftp is gone after its retry rather than cascading.
The case that still skips the inventory is a real crash in the ftp build
(extension exception, autoapi import error). That one cascades on `main` too:
the rebuild-all pass builds ftp with html, the html builder writes
`objects.inv` in `finish()`, and a crashed build never gets there, so sftp
fails on the label again in passes 2, 3 and 4. Same end state, about an hour
later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
