This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch v3-3-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/v3-3-test by this push:
new 68d603c4ee5 Speed up documentation build and spellcheck in CI (#73477)
(#73550)
68d603c4ee5 is described below
commit 68d603c4ee59745bf39b3726aca6c37b58e45dba
Author: Jarek Potiuk <[email protected]>
AuthorDate: Tue Sep 22 17:34:16 2026 +0200
Speed up documentation build and spellcheck in CI (#73477) (#73550)
Run Sphinx in-process inside each docs build worker so the fixed per-package
startup (autoapi's astroid parsing of the airflow modules, provider.yaml
parsing, the airflow import, inventory loading) is paid once per worker
instead of once per package. A full 115-package build drops from 645 s to
214 s locally with 4 workers (41.6 to 14.2 CPU-minutes); a small provider
now takes about 2 s instead of 17 s.
Make the spelling builder write an inventory so a --spellcheck-only run can
resolve references to pages added in the same change, and retry only the
packages that failed instead of rebuilding everything with html and spelling
again. That rebuild-all pass is what turned a 28-minute spellcheck job into
a
93-minute one.
Also schedule the heaviest packages first with one-at-a-time dispatch, parse
each provider.yaml once with a compiled validator and the C YAML loader, and
skip the runner disk cleanup for the docs job (93 GB were free before it).
Runners are not uniform, so the docs job now passes a 40 GB threshold to
the prepare action instead of skipping the cleanup unconditionally. Also
document why a spelling build that fails on misspellings still publishes its
inventory: under -W Sphinx counts warnings and fails at the end rather than
aborting.
(cherry picked from commit b8be9e54beef9174c95ca37fa5c4d1e0764ab0cb)
Co-authored-by: Kaxil Naik <[email protected]>
---
.../actions/prepare_breeze_and_image/action.yml | 18 +-
.github/workflows/ci-image-checks.yml | 3 +
devel-common/src/docs/build_docs.py | 99 +++++------
devel-common/src/docs/provider_conf.py | 7 +-
.../src/sphinx_exts/airflow_intersphinx.py | 67 +++++--
.../src/sphinx_exts/docs_build/docs_builder.py | 197 +++++++++++++++++----
.../src/sphinx_exts/provider_yaml_utils.py | 42 ++++-
.../docs_build/test_build_docs_scheduling.py | 41 +++++
.../sphinx_exts/docs_build/test_docs_builder.py | 146 +++++++++++++++
.../unit/sphinx_exts/test_airflow_intersphinx.py | 113 ++++++++++++
10 files changed, 625 insertions(+), 108 deletions(-)
diff --git a/.github/actions/prepare_breeze_and_image/action.yml
b/.github/actions/prepare_breeze_and_image/action.yml
index 6e6c4efd205..1815b2a743c 100644
--- a/.github/actions/prepare_breeze_and_image/action.yml
+++ b/.github/actions/prepare_breeze_and_image/action.yml
@@ -34,6 +34,14 @@ inputs:
make-mnt-writeable-and-cleanup:
description: 'Whether to cleanup /mnt'
required: true
+ min-free-space-gb:
+ description: >
+ Skip removing the runner's unused pre-installed tooling (close to three
minutes) when the root
+ filesystem already has at least this many GB free. Empty means always
remove it. Runners are
+ not uniform, so a job whose footprint is only the CI image (docs build)
passes a threshold
+ rather than skipping unconditionally.
+ required: false
+ default: ""
outputs:
host-python-version:
description: Python version used in host
@@ -47,7 +55,15 @@ runs:
if: inputs.make-mnt-writeable-and-cleanup == 'true'
- name: "Free up disk space"
shell: bash
- run: ./scripts/tools/free_up_disk_space.sh
+ env:
+ MIN_FREE_SPACE_GB: ${{ inputs.min-free-space-gb }}
+ run: |
+ FREE_GB=$(df --output=avail --block-size=1G / | tail -1 | tr -d ' ')
+ if [[ -n "${MIN_FREE_SPACE_GB}" && "${FREE_GB}" -ge
"${MIN_FREE_SPACE_GB}" ]]; then
+ echo "${FREE_GB} GB free on / (>= ${MIN_FREE_SPACE_GB} GB), skipping
the disk cleanup"
+ else
+ ./scripts/tools/free_up_disk_space.sh
+ fi
- name: "Install Breeze"
uses: ./.github/actions/breeze
id: breeze
diff --git a/.github/workflows/ci-image-checks.yml
b/.github/workflows/ci-image-checks.yml
index aa5b75d52ed..87afc3c8a1f 100644
--- a/.github/workflows/ci-image-checks.yml
+++ b/.github/workflows/ci-image-checks.yml
@@ -193,6 +193,9 @@ jobs:
python: "${{ inputs.default-python-version }}"
use-uv: ${{ inputs.use-uv }}
make-mnt-writeable-and-cleanup: true
+ # The docs build needs ~25 GB (image tar, loaded image, built docs);
runners usually
+ # start with ~90 GB free, in which case the 3-minute cleanup is
skipped.
+ min-free-space-gb: 40
- name: "Restore docs inventory cache"
uses:
apache/infrastructure-actions/stash/restore@ec1b354a0455b41001d77473236d02a0ee0adba4
with:
diff --git a/devel-common/src/docs/build_docs.py
b/devel-common/src/docs/build_docs.py
index 860df0115c7..54ddeb3102b 100755
--- a/devel-common/src/docs/build_docs.py
+++ b/devel-common/src/docs/build_docs.py
@@ -38,7 +38,7 @@ from rich.console import Console
from tabulate import tabulate
from sphinx_exts.docs_build import dev_index_generator
-from sphinx_exts.docs_build.code_utils import CONSOLE_WIDTH, GENERATED_PATH
+from sphinx_exts.docs_build.code_utils import AIRFLOW_CONTENT_ROOT_PATH,
CONSOLE_WIDTH, GENERATED_PATH
from sphinx_exts.docs_build.docs_builder import (
AirflowDocsBuilder,
get_available_packages,
@@ -328,6 +328,34 @@ def print_build_output(result: BuildDocsResult):
console.print(f"[bright_blue]{result.package_name:60}: " + "#" * 80)
+_API_SOURCE_ROOTS = {
+ "apache-airflow": AIRFLOW_CONTENT_ROOT_PATH / "airflow-core" / "src",
+ "task-sdk": AIRFLOW_CONTENT_ROOT_PATH / "task-sdk" / "src",
+ "apache-airflow-ctl": AIRFLOW_CONTENT_ROOT_PATH / "airflow-ctl" / "src",
+}
+
+
+def _estimated_build_weight(package_name: str) -> int:
+ """Number of Python files autoapi will parse: the rough cost of building
one package."""
+ builder = AirflowDocsBuilder(package_name=package_name)
+ source_root = (
+ builder.provider_path / "src" if builder.is_provider else
_API_SOURCE_ROOTS.get(package_name)
+ )
+ if source_root is None or not source_root.exists():
+ return 0
+ return sum(1 for _ in source_root.rglob("*.py"))
+
+
+def sort_heaviest_first(packages: list[str]) -> list[str]:
+ """
+ Order packages so the long builds start first.
+
+ With a handful of workers, starting google/amazon/apache-airflow last
would leave the other workers
+ idle at the end; starting them first lets the many small packages fill in
around them.
+ """
+ return sorted(packages, key=lambda package_name:
(-_estimated_build_weight(package_name), package_name))
+
+
def run_docs_build_in_parallel(
all_build_errors: dict[str, list[DocBuildError]],
packages_to_build: list[str],
@@ -337,7 +365,7 @@ def run_docs_build_in_parallel(
"""Runs documentation building in parallel."""
doc_build_specifications: list[BuildSpecification] = []
with with_group("Scheduling documentation to build"):
- for package_name in packages_to_build:
+ for package_name in sort_heaviest_first(packages_to_build):
console.print(f"[bright_blue]{package_name:60}:[/] Scheduling
documentation to build")
doc_build_specifications.append(
BuildSpecification(
@@ -348,7 +376,11 @@ def run_docs_build_in_parallel(
)
with with_group("Running docs building"):
console.print()
- result_list = pool.map(perform_docs_build_for_single_package,
doc_build_specifications)
+ # chunksize=1 hands packages out one at a time, so a worker that
finishes early picks up the
+ # next package instead of the fixed chunk pool.map would have
pre-assigned to it.
+ result_list = list(
+ pool.imap_unordered(perform_docs_build_for_single_package,
doc_build_specifications, chunksize=1)
+ )
for result in result_list:
if result.errors:
all_build_errors[result.package_name].extend(result.errors)
@@ -377,14 +409,18 @@ def run_spell_check_in_parallel(
"""Runs spell check in parallel."""
spell_check_specifications: list[BuildSpecification] = []
with with_group("Scheduling spell checking of documentation"):
- for package_name in packages_to_build:
+ for package_name in sort_heaviest_first(packages_to_build):
console.print(f"[bright_blue]{package_name:60}:[/] Scheduling
spellchecking")
spell_check_specifications.append(
BuildSpecification(package_name=package_name,
is_autobuild=False, verbose=verbose)
)
with with_group("Running spell checking of documentation"):
console.print()
- result_list = pool.map(perform_spell_check_for_single_package,
spell_check_specifications)
+ result_list = list(
+ pool.imap_unordered(
+ perform_spell_check_for_single_package,
spell_check_specifications, chunksize=1
+ )
+ )
for result in result_list:
if result.spelling_errors:
all_spelling_errors[result.package_name].extend(result.spelling_errors)
@@ -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,
- )
-
- # And try again in case one change spans across three-level
dependencies.
- 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,
- # In the 3rd pass we only rebuild packages that failed in the 2nd
pass
- # no matter if we do spellcheck-only build
- rebuild_all_packages=False,
- )
-
- if spellcheck_only:
- # And in case of spellcheck-only, we add a 4th pass to account for
A->B-C case
- # For spellcheck-only build, the first pass does not solve any of
the dependency
- # Issues, they only start getting solved and the 2nd pass so we
might need to do one more pass
+ # Packages that failed on a cross-reference to a package built later
in the same pass are
+ # built again now that the inventory exists (spelling builds write one
too, see
+ # airflow_intersphinx). The second retry covers a change spanning A ->
B -> C dependencies.
+ for _ in range(2):
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,
+ spellcheck_only=spellcheck_only,
jobs=jobs,
verbose=verbose,
package_build_errors=package_build_errors,
- originally_built_packages=packages_to_build,
- # In the 4th pass we only rebuild packages that failed in the
3rd pass
- # no matter if we do spellcheck-only build
- rebuild_all_packages=False,
)
dev_index_generator.generate_index(f"{GENERATED_PATH}/_build/index.html")
@@ -758,11 +761,10 @@ def retry_building_docs_if_needed(
all_spelling_errors: dict[str, list[SpellingError]],
autobuild: bool,
docs_only: bool,
+ spellcheck_only: bool,
jobs: int,
verbose: bool,
package_build_errors: dict[str, list[DocBuildError]],
- originally_built_packages: list[str],
- rebuild_all_packages: bool,
) -> dict[str, list[DocBuildError]]:
to_retry_packages = [
package_name
@@ -773,11 +775,6 @@ def retry_building_docs_if_needed(
console.print("[green]No packages to retry. No more passes are
needed.[/]")
return package_build_errors
console.print("[warning] Some packages failed to build due to
dependencies. We need another pass.[/]")
- # if we are rebuilding all packages, we need to retry all packages
- # even if there is one package to rebuild only
- if rebuild_all_packages:
- console.print("[warning]Rebuilding all originally built package as
this is the first build pass:[/]")
- to_retry_packages = originally_built_packages
console.print(f"[bright_blue]Packages to rebuild: {to_retry_packages}[/]")
for package_name in to_retry_packages:
if package_name in all_build_errors:
@@ -788,7 +785,7 @@ def retry_building_docs_if_needed(
packages_to_build=to_retry_packages,
is_autobuild=autobuild,
docs_only=docs_only,
- spellcheck_only=False,
+ spellcheck_only=spellcheck_only,
jobs=jobs,
verbose=verbose,
)
diff --git a/devel-common/src/docs/provider_conf.py
b/devel-common/src/docs/provider_conf.py
index 96d5e2f93cc..4df107b7d41 100644
--- a/devel-common/src/docs/provider_conf.py
+++ b/devel-common/src/docs/provider_conf.py
@@ -293,7 +293,12 @@ autoapi_dirs = [BASE_PROVIDER_SRC_PATH.as_posix()]
autoapi_ignore = BASIC_AUTOAPI_IGNORE_PATTERNS
autoapi_log = logging.getLogger("sphinx.autoapi.mappers.base")
-autoapi_log.addFilter(filter_autoapi_ignore_entries)
+# One process builds several packages in a row and re-imports this module each
time, so guard
+# against stacking the same filter on the (process-wide) logger.
+if not any(
+ getattr(f, "__name__", None) == filter_autoapi_ignore_entries.__name__ for
f in autoapi_log.filters
+):
+ autoapi_log.addFilter(filter_autoapi_ignore_entries)
autoapi_python_use_implicit_namespaces = True
diff --git a/devel-common/src/sphinx_exts/airflow_intersphinx.py
b/devel-common/src/sphinx_exts/airflow_intersphinx.py
index 41f8d36659b..a5b83bfcff4 100644
--- a/devel-common/src/sphinx_exts/airflow_intersphinx.py
+++ b/devel-common/src/sphinx_exts/airflow_intersphinx.py
@@ -22,12 +22,61 @@ from pathlib import Path
from typing import TYPE_CHECKING, Any
from provider_yaml_utils import load_package_data
+from sphinx.util.inventory import InventoryFile
if TYPE_CHECKING:
from sphinx.application import Sphinx
AIRFLOW_ROOT_PATH = Path(os.path.abspath(__file__)).parents[3]
GENERATED_PATH = AIRFLOW_ROOT_PATH / "generated"
+INVENTORY_FILENAME = "objects.inv"
+
+
+def _inventory_for(package_name: str, versioned: bool) -> Path:
+ """
+ Prefer an inventory written earlier in this run, fall back to the one
fetched from the published docs.
+
+ The html builder writes its inventory at the root of the build directory;
a spelling-only build
+ writes one into its own output directory (see
``_dump_inventory_after_spelling_build``, the
+ directory name mirrors ``AirflowDocsBuilder.log_spelling_output_dir``).
+ """
+ build_dir = GENERATED_PATH / "_build" / "docs" / package_name / ("stable"
if versioned else "")
+ built_this_run = (
+ build_dir / INVENTORY_FILENAME,
+ build_dir / f"output-spelling-results-{package_name}" /
INVENTORY_FILENAME,
+ )
+ downloaded = GENERATED_PATH / "_inventory_cache" / package_name /
INVENTORY_FILENAME
+ return next((inventory for inventory in built_this_run if
inventory.exists()), downloaded)
+
+
+class _HtmlTargetUris:
+ """Gives inventory entries the URIs the html build would, so the file is
an ordinary inventory."""
+
+ @staticmethod
+ def get_target_uri(docname: str, typ: str | None = None) -> str:
+ return f"{docname}.html"
+
+
+def _dump_inventory_after_spelling_build(app: Sphinx, exception: Exception |
None) -> None:
+ """
+ Write ``objects.inv`` after a successful spelling build.
+
+ Cross-references are resolved by every builder, so a ``--spellcheck-only``
run of package B fails
+ on a label that package A added in the same change unless A's fresh
inventory is available. Only
+ the html builder writes inventories, which used to force the spellcheck
run to rebuild every
+ package a second time with html. Writing the inventory here lets a
spellcheck-only run resolve
+ references the same way a docs build does.
+
+ "Successful" means the build ran to completion, not that it passed: under
``-W`` Sphinx (8.1 and
+ 9.x) counts warnings and sets a non-zero exit status at the end instead of
aborting, so a package
+ with a genuine misspelling or an unresolved reference still reaches this
hook with
+ ``exception=None`` and still publishes its complete inventory for the
packages that depend on it.
+ Only a build that crashed part-way (``exception`` set) is skipped, because
its environment may
+ not hold every document.
+ """
+ if exception is not None or app.builder.name != "spelling":
+ return
+ InventoryFile.dump(os.path.join(app.outdir, INVENTORY_FILENAME), app.env,
_HtmlTargetUris()) # type: ignore[arg-type]
def _create_init_py(app, config):
@@ -51,39 +100,32 @@ def _generate_provider_intersphinx_mapping() -> dict[str,
tuple[str, tuple[str,
continue
provider_base_url = f"/docs/{package_name}/{current_version}/"
- doc_inventory = GENERATED_PATH / "_build" / "docs" / package_name /
current_version / "objects.inv"
- cache_inventory = GENERATED_PATH / "_inventory_cache" / package_name /
"objects.inv"
+ inventory = _inventory_for(package_name, versioned=True)
# Skip adding the mapping if the path does not exist
- if not os.path.exists(doc_inventory) and not
os.path.exists(cache_inventory):
+ if not inventory.exists():
continue
airflow_mapping[package_name] = (
# base URI
provider_base_url,
- (doc_inventory.as_posix() if doc_inventory.exists() else
cache_inventory.as_posix(),),
+ (inventory.as_posix(),),
)
for pkg_name in ["apache-airflow", "helm-chart", "task-sdk"]:
if os.environ.get("AIRFLOW_PACKAGE_NAME") == pkg_name:
continue
- doc_inventory = GENERATED_PATH / "_build" / "docs" / pkg_name /
current_version / "objects.inv"
- cache_inventory = GENERATED_PATH / "_inventory_cache" / pkg_name /
"objects.inv"
-
airflow_mapping[pkg_name] = (
# base URI
f"/docs/{pkg_name}/stable/",
- (doc_inventory.as_posix() if doc_inventory.exists() else
cache_inventory.as_posix(),),
+ (_inventory_for(pkg_name, versioned=True).as_posix(),),
)
for pkg_name in ["apache-airflow-providers", "docker-stack"]:
if os.environ.get("AIRFLOW_PACKAGE_NAME") == pkg_name:
continue
- doc_inventory = GENERATED_PATH / "_build" / "docs" / pkg_name /
"objects.inv"
- cache_inventory = GENERATED_PATH / "_inventory_cache" / pkg_name /
"objects.inv"
-
airflow_mapping[pkg_name] = (
# base URI
f"/docs/{pkg_name}/",
- (doc_inventory.as_posix() if doc_inventory.exists() else
cache_inventory.as_posix(),),
+ (_inventory_for(pkg_name, versioned=False).as_posix(),),
)
return airflow_mapping
@@ -91,6 +133,7 @@ def _generate_provider_intersphinx_mapping() -> dict[str,
tuple[str, tuple[str,
def setup(app: Sphinx):
"""Sets the plugin up"""
app.connect("config-inited", _create_init_py)
+ app.connect("build-finished", _dump_inventory_after_spelling_build)
return {"version": "builtin", "parallel_read_safe": True,
"parallel_write_safe": True}
diff --git a/devel-common/src/sphinx_exts/docs_build/docs_builder.py
b/devel-common/src/sphinx_exts/docs_build/docs_builder.py
index 147215a7353..a7df0ad6a10 100644
--- a/devel-common/src/sphinx_exts/docs_build/docs_builder.py
+++ b/devel-common/src/sphinx_exts/docs_build/docs_builder.py
@@ -16,21 +16,27 @@
# under the License.
from __future__ import annotations
+import contextlib
import os
import re
import shlex
import shutil
+import signal
import sys
+import threading
+from collections.abc import Iterator
from pathlib import Path
from subprocess import run
from rich.console import Console
+from sphinx.cmd.build import build_main
from sphinx_exts.docs_build.code_utils import (
AIRFLOW_CONTENT_ROOT_PATH,
ALL_PROVIDER_YAMLS,
ALL_PROVIDER_YAMLS_WITH_SUSPENDED,
CONSOLE_WIDTH,
+ DOCS_SOURCES_PATH,
GENERATED_PATH,
PROCESS_TIMEOUT,
)
@@ -39,6 +45,121 @@ from sphinx_exts.docs_build.spelling_checks import
SpellingError, parse_spelling
console = Console(force_terminal=True, color_system="standard",
width=CONSOLE_WIDTH)
+# Sphinx is run in the current process (one worker builds many packages in a
row) so that the fixed
+# start-up cost of a build is paid once per worker instead of once per
package. For a small provider
+# that fixed cost is ~20s out of ~22s: importing airflow, parsing and
validating every provider.yaml,
+# and - the largest part - autoapi's astroid parsing of the airflow modules
every provider imports.
+# The astroid parse cache in particular stays warm across the packages a
worker builds.
+
+
+def _forget_sphinx_conf_modules() -> None:
+ """
+ Drop the shared Sphinx configuration modules so the next conf.py
re-executes them from scratch.
+
+ The per-package ``conf.py`` files do ``from docs.provider_conf import *``
(or import
+ ``docs.utils.conf_constants``) and then mutate the lists they get -
``extensions.append(...)``,
+ ``autoapi_ignore.extend(...)``. Reusing the cached module would leak one
package's additions into
+ the next build, so everything under ``devel-common/src/docs`` except the
build script is forgotten.
+ """
+ docs_sources_prefix = DOCS_SOURCES_PATH.as_posix()
+ for name, module in list(sys.modules.items()):
+ if name == "docs.build_docs" or not name.startswith("docs."):
+ continue
+ module_file = getattr(module, "__file__", None) or ""
+ if module_file.startswith(docs_sources_prefix):
+ del sys.modules[name]
+
+
+class _RedirectableStream:
+ """
+ Stand-in for ``sys.stdout`` / ``sys.stderr`` that lives as long as the
worker process.
+
+ Libraries keep a reference to whatever stream is current when they first
need one (docutils'
+ ``Reporter`` in sphinx-argparse's nested parser is one example).
Redirecting straight to a
+ per-package log file would leave such references pointing at a closed file
once that package is
+ done and crash the next build with "I/O operation on closed file". This
object never closes; it
+ only changes where it writes: the current package's log file during a
build, the worker's real
+ stream otherwise.
+ """
+
+ def __init__(self, fallback) -> None:
+ self._fallback = fallback
+ self.target = None
+
+ def _stream(self):
+ return self.target if self.target is not None else self._fallback
+
+ def write(self, data: str) -> int:
+ return self._stream().write(data)
+
+ def flush(self) -> None:
+ self._stream().flush()
+
+ def isatty(self) -> bool:
+ return False
+
+ @property
+ def encoding(self) -> str:
+ return getattr(self._stream(), "encoding", None) or "utf-8"
+
+
+_STDOUT_PROXY = _RedirectableStream(sys.stdout)
+_STDERR_PROXY = _RedirectableStream(sys.stderr)
+
+
[email protected]
+def _output_to(log_file) -> Iterator[None]:
+ """Send everything written to stdout/stderr during the block to
``log_file``."""
+ _STDOUT_PROXY.target = log_file
+ _STDERR_PROXY.target = log_file
+ try:
+ with contextlib.redirect_stdout(_STDOUT_PROXY),
contextlib.redirect_stderr(_STDERR_PROXY):
+ yield
+ finally:
+ _STDOUT_PROXY.target = None
+ _STDERR_PROXY.target = None
+
+
[email protected]
+def _working_directory(path: Path) -> Iterator[None]:
+ previous = os.getcwd()
+ os.chdir(path)
+ try:
+ yield
+ finally:
+ os.chdir(previous)
+
+
[email protected]
+def _sys_path_prepended(paths: list[Path]) -> Iterator[None]:
+ entries = [path.as_posix() for path in paths]
+ sys.path[:0] = entries
+ try:
+ yield
+ finally:
+ for entry in entries:
+ with contextlib.suppress(ValueError):
+ sys.path.remove(entry)
+
+
[email protected]
+def _build_timeout(seconds: int) -> Iterator[None]:
+ """Abort a runaway in-process build the way the previous subprocess
timeout did."""
+ if not hasattr(signal, "SIGALRM") or threading.current_thread() is not
threading.main_thread():
+ yield
+ return
+
+ def _raise_timeout(signum, frame):
+ raise TimeoutError(f"Sphinx build did not finish within {seconds}
seconds")
+
+ previous_handler = signal.signal(signal.SIGALRM, _raise_timeout)
+ signal.alarm(seconds)
+ try:
+ yield
+ finally:
+ signal.alarm(0)
+ signal.signal(signal.SIGALRM, previous_handler)
+
class AirflowDocsBuilder:
"""Documentation builder for Airflow."""
@@ -181,25 +302,12 @@ class AirflowDocsBuilder:
]
if os.environ.get("CI", "") != "true" and verbose:
console.print("[yellow]Command to run:[/] ", "
".join([shlex.quote(arg) for arg in build_cmd]))
- env = os.environ.copy()
- env["AIRFLOW_PACKAGE_NAME"] = self.package_name
- if self.pythonpath:
- env["PYTHONPATH"] = ":".join([path.as_posix() for path in
self.pythonpath])
if verbose:
console.print(
f"[bright_blue]{self.package_name:60}:[/] The output is hidden
until an error occurs."
)
- with open(self.log_spelling_filename, "w") as output:
- completed_proc = run(
- build_cmd,
- check=False,
- cwd=AIRFLOW_CONTENT_ROOT_PATH,
- env=env,
- stdout=output if not verbose else None,
- stderr=output if not verbose else None,
- timeout=PROCESS_TIMEOUT,
- )
- if completed_proc.returncode != 0:
+ returncode = self._run_sphinx(build_cmd,
log_file=self.log_spelling_filename, verbose=verbose)
+ if returncode != 0:
spelling_errors.append(
SpellingError(
file_path=None,
@@ -207,9 +315,7 @@ class AirflowDocsBuilder:
spelling=None,
suggestion=None,
context_line=None,
- message=(
- f"Sphinx spellcheck returned non-zero exit status:
{completed_proc.returncode}."
- ),
+ message=f"Sphinx spellcheck returned non-zero exit status:
{returncode}.",
)
)
spelling_warning_text = ""
@@ -263,31 +369,18 @@ class AirflowDocsBuilder:
]
if os.environ.get("CI", "") != "true" and verbose:
console.print("[yellow]Command to run:[/] ", "
".join([shlex.quote(arg) for arg in build_cmd]))
- env = os.environ.copy()
- env["AIRFLOW_PACKAGE_NAME"] = self.package_name
- if self.pythonpath:
- env["PYTHONPATH"] = ":".join([path.as_posix() for path in
self.pythonpath])
if verbose:
console.print(
f"[bright_blue]{self.package_name:60}:[/] Running sphinx. "
f"The output is hidden until an error occurs."
)
- with open(self.log_build_filename, "w") as output:
- completed_proc = run(
- build_cmd,
- check=False,
- cwd=AIRFLOW_CONTENT_ROOT_PATH,
- env=env,
- stdout=output if not verbose else None,
- stderr=output if not verbose else None,
- timeout=PROCESS_TIMEOUT,
- )
- if completed_proc.returncode != 0:
+ returncode = self._run_sphinx(build_cmd,
log_file=self.log_build_filename, verbose=verbose)
+ if returncode != 0:
build_errors.append(
DocBuildError(
file_path=None,
line_no=None,
- message=f"Sphinx returned non-zero exit status:
{completed_proc.returncode}.",
+ message=f"Sphinx returned non-zero exit status:
{returncode}.",
)
)
if self.log_build_warning_filename.is_file():
@@ -308,6 +401,42 @@ class AirflowDocsBuilder:
def get_command(self) -> str:
return "sphinx-autobuild" if self.is_autobuild else "sphinx-build"
+ def _run_sphinx(self, build_cmd: list[str], *, log_file: Path, verbose:
bool) -> int:
+ """
+ Run a ``sphinx-build`` / ``sphinx-autobuild`` command line and return
its exit status.
+
+ ``sphinx-build`` runs in the current process (see the module comment
for why); its output goes
+ to ``log_file`` unless ``verbose`` is set. ``sphinx-autobuild`` is a
long-running server and
+ keeps running as a subprocess.
+ """
+ if self.is_autobuild:
+ env = os.environ.copy()
+ env["AIRFLOW_PACKAGE_NAME"] = self.package_name
+ if self.pythonpath:
+ env["PYTHONPATH"] = ":".join([path.as_posix() for path in
self.pythonpath])
+ with open(log_file, "w") as output:
+ completed_proc = run(
+ build_cmd,
+ check=False,
+ cwd=AIRFLOW_CONTENT_ROOT_PATH,
+ env=env,
+ stdout=output if not verbose else None,
+ stderr=output if not verbose else None,
+ timeout=PROCESS_TIMEOUT,
+ )
+ return completed_proc.returncode
+ _forget_sphinx_conf_modules()
+ os.environ["AIRFLOW_PACKAGE_NAME"] = self.package_name
+ with (
+ open(log_file, "w") as output,
+ _output_to(output) if not verbose else contextlib.nullcontext(),
+ _working_directory(AIRFLOW_CONTENT_ROOT_PATH),
+ _sys_path_prepended(self.pythonpath),
+ _build_timeout(PROCESS_TIMEOUT),
+ ):
+ # Sphinx reports a TimeoutError raised by the alarm like any other
build failure.
+ return build_main(build_cmd[1:])
+
def get_available_providers_distributions(include_suspended: bool = False):
"""Get list of all available providers packages to build."""
diff --git a/devel-common/src/sphinx_exts/provider_yaml_utils.py
b/devel-common/src/sphinx_exts/provider_yaml_utils.py
index 9fc02b61f3c..324876be6fd 100644
--- a/devel-common/src/sphinx_exts/provider_yaml_utils.py
+++ b/devel-common/src/sphinx_exts/provider_yaml_utils.py
@@ -23,6 +23,7 @@ from pathlib import Path
from typing import Any
import jsonschema
+import jsonschema.validators
import yaml
AIRFLOW_ROOT_PATH = Path(__file__).parents[3].resolve()
@@ -31,6 +32,8 @@ AIRFLOW_PROVIDERS_SRC = AIRFLOW_PROVIDERS_PATH / "src"
PROVIDER_DATA_SCHEMA_PATH = (
AIRFLOW_ROOT_PATH / "airflow-core" / "src" / "airflow" /
"provider.yaml.schema.json"
)
+# The C loader parses YAML several times faster than the pure-Python one; fall
back when libyaml is absent.
+_YAML_LOADER = getattr(yaml, "CSafeLoader", yaml.SafeLoader)
@cache
@@ -65,24 +68,31 @@ def get_all_provider_yaml_paths() -> list[Path]:
@cache
-def load_package_data(include_suspended: bool = False) -> list[dict[str, Any]]:
- """
- Load all data from providers files
+def _provider_yaml_validator() -> jsonschema.protocols.Validator:
+ """Return a validator compiled once for the provider.yaml schema.
- :return: A list containing the contents of all provider.yaml files - old
and new structure.
+ ``jsonschema.validate`` re-validates the schema itself on every call,
which is the dominant
+ cost when validating ~100 provider.yaml files in a row (several seconds
per Sphinx process).
"""
schema = provider_yaml_schema()
+ validator_cls = jsonschema.validators.validator_for(schema)
+ validator_cls.check_schema(schema)
+ return validator_cls(schema)
+
+
+@cache
+def _load_all_provider_yamls() -> tuple[dict[str, Any], ...]:
+ """Parse and validate every provider.yaml once per process, suspended
providers included."""
+ validator = _provider_yaml_validator()
result = []
for provider_yaml_path in get_all_provider_yaml_paths():
with open(provider_yaml_path) as yaml_file:
- provider = yaml.safe_load(yaml_file)
+ provider = yaml.load(yaml_file, Loader=_YAML_LOADER)
try:
- jsonschema.validate(provider, schema=schema)
+ validator.validate(provider)
except jsonschema.ValidationError as ex:
msg = f"Unable to parse: {provider_yaml_path}. Original error
{type(ex).__name__}: {ex}"
raise RuntimeError(msg)
- if provider["state"] == "suspended" and not include_suspended:
- continue
provider_yaml_dir_str = os.path.dirname(provider_yaml_path)
module = provider["package-name"][len("apache-") :].replace("-", ".")
module_folder = module[len("airflow-providers-") :].replace(".", "/")
@@ -91,4 +101,18 @@ def load_package_data(include_suspended: bool = False) ->
list[dict[str, Any]]:
provider["docs-dir"] = os.path.dirname(provider_yaml_path.parent /
"docs")
provider["system-tests-dir"] =
f"{provider_yaml_dir_str}/tests/system/{module_folder}"
result.append(provider)
- return result
+ return tuple(result)
+
+
+@cache
+def load_package_data(include_suspended: bool = False) -> list[dict[str, Any]]:
+ """
+ Load all data from providers files
+
+ :return: A list containing the contents of all provider.yaml files - old
and new structure.
+ """
+ return [
+ provider
+ for provider in _load_all_provider_yamls()
+ if include_suspended or provider["state"] != "suspended"
+ ]
diff --git
a/devel-common/tests/unit/sphinx_exts/docs_build/test_build_docs_scheduling.py
b/devel-common/tests/unit/sphinx_exts/docs_build/test_build_docs_scheduling.py
new file mode 100644
index 00000000000..5e827401061
--- /dev/null
+++
b/devel-common/tests/unit/sphinx_exts/docs_build/test_build_docs_scheduling.py
@@ -0,0 +1,41 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+from unittest import mock
+
+from docs import build_docs
+
+
[email protected](build_docs, "_estimated_build_weight", autospec=True)
+def test_sort_heaviest_first_orders_by_weight_then_name(mock_weight):
+ weights = {"google": 3000, "amazon": 1500, "ftp": 20, "ssh": 20,
"docker-stack": 5}
+ mock_weight.side_effect = weights.__getitem__
+
+ ordered = build_docs.sort_heaviest_first(["ssh", "docker-stack", "google",
"ftp", "amazon"])
+
+ assert ordered == ["google", "amazon", "ftp", "ssh", "docker-stack"]
+
+
+def test_estimated_build_weight_ranks_real_packages_sensibly():
+ google =
build_docs._estimated_build_weight("apache-airflow-providers-google")
+ core = build_docs._estimated_build_weight("apache-airflow")
+ ftp = build_docs._estimated_build_weight("apache-airflow-providers-ftp")
+
+ assert google > ftp > 0
+ assert core > ftp
+ assert build_docs._estimated_build_weight("docker-stack") == 0
diff --git
a/devel-common/tests/unit/sphinx_exts/docs_build/test_docs_builder.py
b/devel-common/tests/unit/sphinx_exts/docs_build/test_docs_builder.py
new file mode 100644
index 00000000000..bd68b2af52d
--- /dev/null
+++ b/devel-common/tests/unit/sphinx_exts/docs_build/test_docs_builder.py
@@ -0,0 +1,146 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import io
+import os
+import subprocess
+import sys
+import types
+from unittest import mock
+
+from sphinx_exts.docs_build import docs_builder
+from sphinx_exts.docs_build.code_utils import AIRFLOW_CONTENT_ROOT_PATH,
DOCS_SOURCES_PATH
+from sphinx_exts.docs_build.docs_builder import (
+ AirflowDocsBuilder,
+ _forget_sphinx_conf_modules,
+ _RedirectableStream,
+)
+
+
+class TestRedirectableStream:
+ def test_writes_go_to_target_while_set_and_to_fallback_otherwise(self):
+ fallback = io.StringIO()
+ target = io.StringIO()
+ stream = _RedirectableStream(fallback)
+
+ stream.write("before\n")
+ stream.target = target
+ stream.write("during\n")
+ stream.target = None
+ stream.write("after\n")
+
+ assert fallback.getvalue() == "before\nafter\n"
+ assert target.getvalue() == "during\n"
+
+ def
test_reference_kept_from_an_earlier_build_stays_usable_after_its_log_is_closed(self):
+ """A library that cached the stream during build 1 must not hit a
closed file in build 2."""
+ fallback = io.StringIO()
+ stream = _RedirectableStream(fallback)
+ first_log = io.StringIO()
+ stream.target = first_log
+ cached_by_library = stream
+ stream.target = None
+ first_log.close()
+ second_log = io.StringIO()
+ stream.target = second_log
+
+ cached_by_library.write("from build 2")
+
+ assert second_log.getvalue() == "from build 2"
+ assert stream.isatty() is False
+ assert stream.encoding
+
+
+class TestForgetSphinxConfModules:
+ def
test_forgets_conf_modules_but_keeps_build_script_and_unrelated_modules(self,
monkeypatch):
+ conf_module = types.ModuleType("docs.provider_conf")
+ conf_module.__file__ = (DOCS_SOURCES_PATH /
"provider_conf.py").as_posix()
+ constants_module = types.ModuleType("docs.utils.conf_constants")
+ constants_module.__file__ = (DOCS_SOURCES_PATH / "utils" /
"conf_constants.py").as_posix()
+ build_script = types.ModuleType("docs.build_docs")
+ build_script.__file__ = (DOCS_SOURCES_PATH /
"build_docs.py").as_posix()
+ elsewhere = types.ModuleType("docs.elsewhere")
+ elsewhere.__file__ = "/somewhere/else/docs/elsewhere.py"
+ for module in (conf_module, constants_module, build_script, elsewhere):
+ monkeypatch.setitem(sys.modules, module.__name__, module)
+
+ _forget_sphinx_conf_modules()
+
+ assert "docs.provider_conf" not in sys.modules
+ assert "docs.utils.conf_constants" not in sys.modules
+ assert sys.modules["docs.build_docs"] is build_script
+ assert sys.modules["docs.elsewhere"] is elsewhere
+
+
+class TestRunSphinxInProcess:
+ def
test_runs_build_main_with_output_in_log_file_and_restores_process_state(self,
tmp_path, monkeypatch):
+ builder =
AirflowDocsBuilder(package_name="apache-airflow-providers-ftp")
+ log_file = tmp_path / "build.log"
+ previous_cwd = os.getcwd()
+ previous_sys_path = list(sys.path)
+ seen: dict[str, object] = {}
+
+ def fake_build_main(argv):
+ seen["argv"] = list(argv)
+ seen["cwd"] = os.getcwd()
+ seen["package"] = os.environ.get("AIRFLOW_PACKAGE_NAME")
+ print("sphinx says hi")
+ print("sphinx warns", file=sys.stderr)
+ return 0
+
+ monkeypatch.setattr(
+ docs_builder,
+ "build_main",
+ mock.create_autospec(docs_builder.build_main,
side_effect=fake_build_main),
+ )
+ monkeypatch.setattr(
+ docs_builder,
+ "_forget_sphinx_conf_modules",
+ mock.create_autospec(docs_builder._forget_sphinx_conf_modules),
+ )
+
+ returncode = builder._run_sphinx(
+ ["sphinx-build", "-T", "-b", "html", "src", "out"],
log_file=log_file, verbose=False
+ )
+
+ assert returncode == 0
+ assert seen["argv"] == ["-T", "-b", "html", "src", "out"]
+ assert seen["cwd"] == AIRFLOW_CONTENT_ROOT_PATH.as_posix()
+ assert seen["package"] == "apache-airflow-providers-ftp"
+ assert log_file.read_text() == "sphinx says hi\nsphinx warns\n"
+ assert os.getcwd() == previous_cwd
+ assert sys.path == previous_sys_path
+ docs_builder._forget_sphinx_conf_modules.assert_called_once_with()
+
+ def test_autobuild_stays_a_subprocess(self, tmp_path, monkeypatch):
+ builder =
AirflowDocsBuilder(package_name="apache-airflow-providers-ftp")
+ builder.is_autobuild = True
+ run = mock.create_autospec(
+ docs_builder.run,
return_value=mock.Mock(spec=subprocess.CompletedProcess, returncode=3)
+ )
+ monkeypatch.setattr(docs_builder, "run", run)
+ monkeypatch.setattr(docs_builder, "build_main",
mock.create_autospec(docs_builder.build_main))
+
+ returncode = builder._run_sphinx(
+ ["sphinx-autobuild", "src", "out"], log_file=tmp_path / "log",
verbose=False
+ )
+
+ assert returncode == 3
+ run.assert_called_once()
+ assert run.call_args.args[0] == ["sphinx-autobuild", "src", "out"]
+ docs_builder.build_main.assert_not_called()
diff --git a/devel-common/tests/unit/sphinx_exts/test_airflow_intersphinx.py
b/devel-common/tests/unit/sphinx_exts/test_airflow_intersphinx.py
new file mode 100644
index 00000000000..ce5b77ad9ba
--- /dev/null
+++ b/devel-common/tests/unit/sphinx_exts/test_airflow_intersphinx.py
@@ -0,0 +1,113 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+from __future__ import annotations
+
+import sys
+from pathlib import Path
+from unittest import mock
+
+import pytest
+from sphinx.application import Sphinx
+from sphinx.builders import Builder
+from sphinx.environment import BuildEnvironment
+from sphinx.errors import SphinxError
+
+SPHINX_EXTS_PATH = Path(__file__).parents[3] / "src" / "sphinx_exts"
+if SPHINX_EXTS_PATH.as_posix() not in sys.path:
+ # The extensions are loaded by Sphinx from this directory and import each
other by bare name.
+ sys.path.append(SPHINX_EXTS_PATH.as_posix())
+
+from sphinx_exts import airflow_intersphinx # noqa: E402
+
+PACKAGE = "apache-airflow-providers-ftp"
+
+
[email protected]
+def generated_path(tmp_path, monkeypatch):
+ monkeypatch.setattr(airflow_intersphinx, "GENERATED_PATH", tmp_path)
+ return tmp_path
+
+
+def _touch(path: Path) -> Path:
+ path.parent.mkdir(parents=True, exist_ok=True)
+ path.write_bytes(b"")
+ return path
+
+
+class TestInventoryFor:
+ def test_prefers_the_html_inventory_built_in_this_run(self,
generated_path):
+ build_dir = generated_path / "_build" / "docs" / PACKAGE / "stable"
+ html = _touch(build_dir / "objects.inv")
+ _touch(build_dir / f"output-spelling-results-{PACKAGE}" /
"objects.inv")
+ _touch(generated_path / "_inventory_cache" / PACKAGE / "objects.inv")
+
+ assert airflow_intersphinx._inventory_for(PACKAGE, versioned=True) ==
html
+
+ def test_uses_the_spelling_inventory_when_only_a_spelling_build_ran(self,
generated_path):
+ build_dir = generated_path / "_build" / "docs" / PACKAGE / "stable"
+ spelling = _touch(build_dir / f"output-spelling-results-{PACKAGE}" /
"objects.inv")
+ _touch(generated_path / "_inventory_cache" / PACKAGE / "objects.inv")
+
+ assert airflow_intersphinx._inventory_for(PACKAGE, versioned=True) ==
spelling
+
+ def test_falls_back_to_the_downloaded_inventory(self, generated_path):
+ cached = generated_path / "_inventory_cache" / PACKAGE / "objects.inv"
+
+ assert airflow_intersphinx._inventory_for(PACKAGE, versioned=True) ==
cached
+
+ def test_non_versioned_packages_have_no_stable_directory(self,
generated_path):
+ html = _touch(generated_path / "_build" / "docs" / "docker-stack" /
"objects.inv")
+
+ assert airflow_intersphinx._inventory_for("docker-stack",
versioned=False) == html
+
+
+class TestDumpInventoryAfterSpellingBuild:
+ @staticmethod
+ def _app(builder_name: str, outdir: Path) -> mock.Mock:
+ app = mock.Mock(spec=Sphinx)
+ app.builder = mock.Mock(spec=Builder)
+ app.builder.name = builder_name
+ app.outdir = outdir
+ app.env = mock.Mock(spec=BuildEnvironment)
+ return app
+
+ @mock.patch.object(airflow_intersphinx.InventoryFile, "dump",
autospec=True)
+ def test_writes_an_inventory_into_the_spelling_output_dir(self, mock_dump,
tmp_path):
+ app = self._app("spelling", tmp_path)
+
+ airflow_intersphinx._dump_inventory_after_spelling_build(app, None)
+
+ mock_dump.assert_called_once()
+ filename, env, uri_builder = mock_dump.call_args.args
+ assert filename == (tmp_path / "objects.inv").as_posix()
+ assert env is app.env
+ assert uri_builder.get_target_uri("operators/index") ==
"operators/index.html"
+
+ @pytest.mark.parametrize(
+ ("builder_name", "exception"),
+ [
+ pytest.param("html", None, id="html-builder-writes-its-own"),
+ pytest.param("spelling", SphinxError("boom"), id="failed-build"),
+ ],
+ )
+ @mock.patch.object(airflow_intersphinx.InventoryFile, "dump",
autospec=True)
+ def test_skips_other_builders_and_failed_builds(self, mock_dump, tmp_path,
builder_name, exception):
+ app = self._app(builder_name, tmp_path)
+
+ airflow_intersphinx._dump_inventory_after_spelling_build(app,
exception)
+
+ mock_dump.assert_not_called()
