From 70ff0739b910236149178386d76284856c9929eb Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 17 Sep 2026 14:20:53 -0700 Subject: [PATCH 01/14] chore: open issues on docfx failures --- .github/workflows/docs.yml | 94 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 89 insertions(+), 5 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index ecc637d802ab..598e7b081dba 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -15,7 +15,32 @@ name: docs permissions: contents: read +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +env: + NOX_DEFAULT_VENV_BACKEND: "uv" + UV_VENV_SEED: "1" + PARALLEL_WORKERS: "4" + jobs: + check_changes: + if: github.event_name == 'pull_request' + runs-on: ubuntu-latest + outputs: + run_docfx: ${{ steps.filter.outputs.docfx }} + steps: + - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 + with: + persist-credentials: false + - uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1 + id: filter + with: + filters: | + docfx: + - '.github/workflows/docs.yml' + docs: runs-on: ubuntu-latest steps: @@ -34,19 +59,35 @@ jobs: - name: Install nox run: | python -m pip install --upgrade setuptools pip wheel - python -m pip install nox + python -m pip install nox uv - name: Run docs env: - BUILD_TYPE: presubmit + BUILD_TYPE: ${{ github.event_name == 'push' && 'continuous' || 'presubmit' }} TARGET_BRANCH: ${{ github.base_ref || github.event.merge_group.base_ref || github.ref_name }} TEST_TYPE: docs # TODO(https://github.com/googleapis/google-cloud-python/issues/13775): Specify `PY_VERSION` rather than relying on the default python version of the nox session. PY_VERSION: "unused" run: | + if [[ "${{ github.event_name }}" == "pull_request" ]]; then + git fetch --no-tags --quiet origin "${TARGET_BRANCH}:refs/remotes/origin/${TARGET_BRANCH}" --depth=200 || true + if ! git diff --quiet "origin/${TARGET_BRANCH}..." -- .github/workflows/docs.yml && git diff --quiet "origin/${TARGET_BRANCH}..." -- packages/ preview-packages/; then + echo "Workflow modified with no package changes; testing packages/google-cloud-core." + export PACKAGE_LIST="packages/google-cloud-core" + fi + fi ci/run_conditional_tests.sh + docfx: - if: github.event_name == 'push' && github.ref == 'refs/heads/main' + needs: [check_changes] + if: >- + always() && ( + (github.event_name == 'push' && github.ref == 'refs/heads/main') || + needs.check_changes.outputs.run_docfx == 'true' + ) runs-on: ubuntu-latest + permissions: + contents: read + issues: write steps: - name: Checkout uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 @@ -63,13 +104,56 @@ jobs: - name: Install nox run: | python -m pip install --upgrade setuptools pip wheel - python -m pip install nox + python -m pip install nox uv - name: Run docfx env: - BUILD_TYPE: presubmit + BUILD_TYPE: ${{ github.event_name == 'push' && 'continuous' || 'presubmit' }} TARGET_BRANCH: ${{ github.base_ref || github.event.merge_group.base_ref || github.ref_name }} TEST_TYPE: docfx + CONTINUE_ON_ERROR: "true" # TODO(https://github.com/googleapis/google-cloud-python/issues/13775): Specify `PY_VERSION` rather than relying on the default python version of the nox session. PY_VERSION: "unused" run: | + if [[ "${{ github.event_name }}" == "pull_request" ]]; then + git fetch --no-tags --quiet origin "${TARGET_BRANCH}:refs/remotes/origin/${TARGET_BRANCH}" --depth=200 || true + if ! git diff --quiet "origin/${TARGET_BRANCH}..." -- .github/workflows/docs.yml && git diff --quiet "origin/${TARGET_BRANCH}..." -- packages/ preview-packages/; then + echo "Workflow modified with no package changes; testing packages/google-cloud-core." + export PACKAGE_LIST="packages/google-cloud-core" + fi + fi ci/run_conditional_tests.sh + - name: Create issue on failure + if: ${{ failure() && github.event_name == 'push' && github.repository == 'googleapis/google-cloud-python' }} + uses: googleapis/librarian/.github/actions/create-issue-on-failure@bb2ee61752bd0f5387195b54c509e4d01a52b41d + with: + title: "docfx check failed" + labels: "priority: p1,type: bug" + body: | + The post-submit [docfx check](${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}) failed on commit `${{ github.sha }}`. + + Please investigate the workflow logs to see which package(s) failed to build DocFX YAML. + + docfx-release-blocker: + name: docfx release blocker + if: "github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'autorelease: pending')" + runs-on: ubuntu-latest + permissions: + contents: read + actions: read + issues: read + steps: + - name: Check post-submit docfx health + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + ISSUE_URL=$(gh issue list --repo "${GITHUB_REPOSITORY}" --state open --search '"docfx check failed" in:title' --json url --jq '.[0].url') + if [ -n "${ISSUE_URL}" ]; then + echo "::error::Release blocked due to open docfx failure issue: ${ISSUE_URL}" + exit 1 + fi + + LATEST_RUN=$(gh run list --repo "${GITHUB_REPOSITORY}" --workflow=docs.yml --branch=main --event=push --status=completed --limit=1 --json conclusion,url --jq '.[0]') + if [ "$(echo "$LATEST_RUN" | jq -r '.conclusion')" = "failure" ]; then + echo "::error::Release blocked because latest post-submit docs workflow on main failed: $(echo "$LATEST_RUN" | jq -r '.url')" + exit 1 + fi From f28921835d271ebd62a36298c33300a985a4e649 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 17 Sep 2026 16:33:47 -0700 Subject: [PATCH 02/14] simulate error --- .github/workflows/docs.yml | 11 +++++++++-- packages/google-cloud-core/noxfile.py | 2 ++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 598e7b081dba..928597dd9351 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -123,7 +123,8 @@ jobs: fi ci/run_conditional_tests.sh - name: Create issue on failure - if: ${{ failure() && github.event_name == 'push' && github.repository == 'googleapis/google-cloud-python' }} + # TODO: Restore `github.event_name == 'push' &&` after testing on this PR + if: ${{ failure() && github.repository == 'googleapis/google-cloud-python' }} uses: googleapis/librarian/.github/actions/create-issue-on-failure@bb2ee61752bd0f5387195b54c509e4d01a52b41d with: title: "docfx check failed" @@ -135,7 +136,13 @@ jobs: docfx-release-blocker: name: docfx release blocker - if: "github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'autorelease: pending')" + needs: [check_changes, docfx] + # TODO: Remove `needs.check_changes.outputs.run_docfx == 'true'` after testing on this PR + if: >- + always() && ( + (github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'autorelease: pending')) || + needs.check_changes.outputs.run_docfx == 'true' + ) runs-on: ubuntu-latest permissions: contents: read diff --git a/packages/google-cloud-core/noxfile.py b/packages/google-cloud-core/noxfile.py index 772c0c136b3d..9abc3525de5a 100644 --- a/packages/google-cloud-core/noxfile.py +++ b/packages/google-cloud-core/noxfile.py @@ -224,6 +224,8 @@ def docs(session): def docfx(session): """Build the docfx yaml files for this library.""" + session.error("Intentional failure to test docfx CI check") + session.install("-e", ".") session.install( # We need to pin to specific versions of the `sphinxcontrib-*` packages From 5e8bc7b1c251132414db4d8136b9c5900122b735 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 17 Sep 2026 17:07:36 -0700 Subject: [PATCH 03/14] reverted intentional breakages --- .github/workflows/docs.yml | 11 ++--------- packages/google-cloud-core/noxfile.py | 2 -- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 928597dd9351..598e7b081dba 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -123,8 +123,7 @@ jobs: fi ci/run_conditional_tests.sh - name: Create issue on failure - # TODO: Restore `github.event_name == 'push' &&` after testing on this PR - if: ${{ failure() && github.repository == 'googleapis/google-cloud-python' }} + if: ${{ failure() && github.event_name == 'push' && github.repository == 'googleapis/google-cloud-python' }} uses: googleapis/librarian/.github/actions/create-issue-on-failure@bb2ee61752bd0f5387195b54c509e4d01a52b41d with: title: "docfx check failed" @@ -136,13 +135,7 @@ jobs: docfx-release-blocker: name: docfx release blocker - needs: [check_changes, docfx] - # TODO: Remove `needs.check_changes.outputs.run_docfx == 'true'` after testing on this PR - if: >- - always() && ( - (github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'autorelease: pending')) || - needs.check_changes.outputs.run_docfx == 'true' - ) + if: "github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'autorelease: pending')" runs-on: ubuntu-latest permissions: contents: read diff --git a/packages/google-cloud-core/noxfile.py b/packages/google-cloud-core/noxfile.py index 9abc3525de5a..772c0c136b3d 100644 --- a/packages/google-cloud-core/noxfile.py +++ b/packages/google-cloud-core/noxfile.py @@ -224,8 +224,6 @@ def docs(session): def docfx(session): """Build the docfx yaml files for this library.""" - session.error("Intentional failure to test docfx CI check") - session.install("-e", ".") session.install( # We need to pin to specific versions of the `sphinxcontrib-*` packages From dd96042cbda5df66f5d4341c7cc62c4a6150a3f9 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 17 Sep 2026 17:17:16 -0700 Subject: [PATCH 04/14] set release blocker step to run, but skip on non-releases --- .github/workflows/docs.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 598e7b081dba..2161cf1d7ccc 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -135,7 +135,7 @@ jobs: docfx-release-blocker: name: docfx release blocker - if: "github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'autorelease: pending')" + if: github.event_name != 'push' runs-on: ubuntu-latest permissions: contents: read @@ -143,6 +143,7 @@ jobs: issues: read steps: - name: Check post-submit docfx health + if: "contains(github.event.pull_request.labels.*.name, 'autorelease: pending')" env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: | From 388313deea39ee5e98b1fcd5dd1e890cf2de1acb Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 17 Sep 2026 17:19:48 -0700 Subject: [PATCH 05/14] removed test against api_core --- .github/workflows/docs.yml | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 2161cf1d7ccc..00e08fce745e 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -68,13 +68,6 @@ jobs: # TODO(https://github.com/googleapis/google-cloud-python/issues/13775): Specify `PY_VERSION` rather than relying on the default python version of the nox session. PY_VERSION: "unused" run: | - if [[ "${{ github.event_name }}" == "pull_request" ]]; then - git fetch --no-tags --quiet origin "${TARGET_BRANCH}:refs/remotes/origin/${TARGET_BRANCH}" --depth=200 || true - if ! git diff --quiet "origin/${TARGET_BRANCH}..." -- .github/workflows/docs.yml && git diff --quiet "origin/${TARGET_BRANCH}..." -- packages/ preview-packages/; then - echo "Workflow modified with no package changes; testing packages/google-cloud-core." - export PACKAGE_LIST="packages/google-cloud-core" - fi - fi ci/run_conditional_tests.sh docfx: @@ -114,13 +107,6 @@ jobs: # TODO(https://github.com/googleapis/google-cloud-python/issues/13775): Specify `PY_VERSION` rather than relying on the default python version of the nox session. PY_VERSION: "unused" run: | - if [[ "${{ github.event_name }}" == "pull_request" ]]; then - git fetch --no-tags --quiet origin "${TARGET_BRANCH}:refs/remotes/origin/${TARGET_BRANCH}" --depth=200 || true - if ! git diff --quiet "origin/${TARGET_BRANCH}..." -- .github/workflows/docs.yml && git diff --quiet "origin/${TARGET_BRANCH}..." -- packages/ preview-packages/; then - echo "Workflow modified with no package changes; testing packages/google-cloud-core." - export PACKAGE_LIST="packages/google-cloud-core" - fi - fi ci/run_conditional_tests.sh - name: Create issue on failure if: ${{ failure() && github.event_name == 'push' && github.repository == 'googleapis/google-cloud-python' }} From 8067503977bf4b6708b5bc6cb311240a68206810 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Thu, 17 Sep 2026 17:23:25 -0700 Subject: [PATCH 06/14] disabled runs when yaml was touched --- .github/workflows/docs.yml | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 00e08fce745e..566c831d8e9e 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -25,23 +25,8 @@ env: PARALLEL_WORKERS: "4" jobs: - check_changes: - if: github.event_name == 'pull_request' - runs-on: ubuntu-latest - outputs: - run_docfx: ${{ steps.filter.outputs.docfx }} - steps: - - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 - with: - persist-credentials: false - - uses: dorny/paths-filter@fbd0ab8f3e69293af611ebaee6363fc25e6d187d # v4.0.1 - id: filter - with: - filters: | - docfx: - - '.github/workflows/docs.yml' - docs: + if: github.event_name != 'push' runs-on: ubuntu-latest steps: - name: Checkout @@ -62,7 +47,7 @@ jobs: python -m pip install nox uv - name: Run docs env: - BUILD_TYPE: ${{ github.event_name == 'push' && 'continuous' || 'presubmit' }} + BUILD_TYPE: presubmit TARGET_BRANCH: ${{ github.base_ref || github.event.merge_group.base_ref || github.ref_name }} TEST_TYPE: docs # TODO(https://github.com/googleapis/google-cloud-python/issues/13775): Specify `PY_VERSION` rather than relying on the default python version of the nox session. @@ -71,12 +56,7 @@ jobs: ci/run_conditional_tests.sh docfx: - needs: [check_changes] - if: >- - always() && ( - (github.event_name == 'push' && github.ref == 'refs/heads/main') || - needs.check_changes.outputs.run_docfx == 'true' - ) + if: github.event_name == 'push' && github.ref == 'refs/heads/main' runs-on: ubuntu-latest permissions: contents: read @@ -100,7 +80,7 @@ jobs: python -m pip install nox uv - name: Run docfx env: - BUILD_TYPE: ${{ github.event_name == 'push' && 'continuous' || 'presubmit' }} + BUILD_TYPE: continuous TARGET_BRANCH: ${{ github.base_ref || github.event.merge_group.base_ref || github.ref_name }} TEST_TYPE: docfx CONTINUE_ON_ERROR: "true" From 74138064211f37a7beccaae8bdb82393f192b64b Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Mon, 5 Oct 2026 23:40:47 -0700 Subject: [PATCH 07/14] added docfx sharding --- .github/workflows/docs.yml | 46 +++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index e26c15d928ab..f3b9a3e2616c 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -24,6 +24,7 @@ env: NOX_ENVDIR: "/tmp/shared_nox_envs" NOX_DEFAULT_VENV_BACKEND: "uv" UV_VENV_SEED: "1" + UV_CONCURRENT_BUILDS: "1" # Run checks in parallel using 4 cores PARALLEL_WORKERS: "4" @@ -41,7 +42,7 @@ jobs: PACKAGE_WEIGHTS: | google-ads-admanager: 9 google-cloud-compute: 50 - google-cloud-compute-v1beta: 100 + google-cloud-compute-v1beta: 120 google-cloud-dialogflow: 9 google-cloud-dialogflow-cx: 9 google-cloud-discoveryengine: 15 @@ -57,7 +58,7 @@ jobs: - name: Check for unit_test:all_packages label id: check-label run: | - if [[ "${{ contains(github.event.pull_request.labels.*.name, 'unit_test:all_packages') }}" == "true" ]]; then + if [[ "${{ github.event_name == 'push' || contains(github.event.pull_request.labels.*.name, 'unit_test:all_packages') }}" == "true" ]]; then echo "is_full_run=true" >> $GITHUB_OUTPUT else echo "is_full_run=false" >> $GITHUB_OUTPUT @@ -83,7 +84,7 @@ jobs: docs-shard: needs: initialize - if: needs.initialize.outputs.matrix != '[]' && needs.initialize.outputs.matrix != '' + if: github.event_name != 'push' && needs.initialize.outputs.matrix != '[]' && needs.initialize.outputs.matrix != '' runs-on: ubuntu-latest strategy: fail-fast: true @@ -119,7 +120,7 @@ jobs: ci/run_conditional_tests.sh docs: - if: always() + if: always() && github.event_name != 'push' needs: [initialize, docs-shard] runs-on: ubuntu-latest name: docs @@ -136,12 +137,15 @@ jobs: fi echo "All docs shards passed or were skipped!" - docfx: - if: github.event_name == 'push' && github.ref == 'refs/heads/main' + docfx-shard: + needs: initialize + if: needs.initialize.outputs.matrix != '[]' && needs.initialize.outputs.matrix != '' runs-on: ubuntu-latest - permissions: - contents: read - issues: write + strategy: + fail-fast: false + matrix: + package_shard: ${{ fromJson(needs.initialize.outputs.matrix) }} + name: ${{ matrix.package_shard.is_sharded && format('docfx ({0})', matrix.package_shard.name) || format('docfx ({0})', matrix.package_shard.description) }} steps: - name: Checkout uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3 @@ -159,16 +163,38 @@ jobs: run: | python -m pip install --upgrade setuptools pip wheel python -m pip install nox uv - - name: Run docfx + - name: Run docfx for ${{ matrix.package_shard.description }} env: BUILD_TYPE: continuous TARGET_BRANCH: ${{ github.base_ref || github.event.merge_group.base_ref || github.ref_name }} TEST_TYPE: docfx + PACKAGE_LIST: ${{ matrix.package_shard.packages }} CONTINUE_ON_ERROR: "true" # TODO(https://github.com/googleapis/google-cloud-python/issues/13775): Specify `PY_VERSION` rather than relying on the default python version of the nox session. PY_VERSION: "unused" run: | ci/run_conditional_tests.sh + + docfx: + if: always() + needs: [initialize, docfx-shard] + runs-on: ubuntu-latest + name: docfx + permissions: + contents: read + issues: write + steps: + - name: Check docfx job status + run: | + if [[ "${{ needs.initialize.result }}" != "success" ]]; then + echo "Error: The initialize job status was: ${{ needs.initialize.result }}" + exit 1 + fi + if [[ "${{ needs['docfx-shard'].result }}" != "success" && "${{ needs['docfx-shard'].result }}" != "skipped" ]]; then + echo "docfx failed with result: ${{ needs['docfx-shard'].result }}" + exit 1 + fi + echo "All docfx shards passed or were skipped!" - name: Create issue on failure if: ${{ failure() && github.event_name == 'push' && github.repository == 'googleapis/google-cloud-python' }} uses: googleapis/librarian/.github/actions/create-issue-on-failure@bb2ee61752bd0f5387195b54c509e4d01a52b41d From 94f4ccdd06fa00424fdad3fab20476cfa13f6541 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Mon, 5 Oct 2026 23:56:20 -0700 Subject: [PATCH 08/14] use cextension when available --- .github/workflows/docs.yml | 9 +++++++++ packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py | 9 ++++++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index f3b9a3e2616c..5e70060d2832 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -163,6 +163,15 @@ jobs: run: | python -m pip install --upgrade setuptools pip wheel python -m pip install nox uv + - name: Build local gcp-sphinx-docfx-yaml wheel + run: | + mkdir -p /tmp/local_wheels + python -m pip wheel --no-deps packages/gcp-sphinx-docfx-yaml -w /tmp/local_wheels + WHEEL_PATH=$(ls /tmp/local_wheels/gcp_sphinx_docfx_yaml-*.whl) + echo "Built local wheel: ${WHEEL_PATH}" + echo "gcp-sphinx-docfx-yaml @ file://${WHEEL_PATH}" > /tmp/uv_overrides.txt + echo "UV_OVERRIDE=/tmp/uv_overrides.txt" >> $GITHUB_ENV + echo "PIP_FIND_LINKS=/tmp/local_wheels" >> $GITHUB_ENV - name: Run docfx for ${{ matrix.package_shard.description }} env: BUILD_TYPE: continuous diff --git a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py index 1d8479df1b6b..a1d6050dac73 100644 --- a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py +++ b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py @@ -46,13 +46,20 @@ import subprocess import sphinx.application +import yaml from docuploader import shell from sphinx.errors import ExtensionError from sphinx.ext.napoleon import Config, GoogleDocstring, _process_docstring from sphinx.util import ensuredir from sphinx.util.console import bold, darkgreen from sphinx.util.nodes import make_refnode -from yaml import safe_dump as dump + +try: + from yaml import CSafeDumper as SafeDumper +except ImportError: + from yaml import SafeDumper + +dump = partial(yaml.dump, Dumper=SafeDumper) from docfx_yaml import markdown_utils From cce364e926b659f774d30426f36136ff222b68a7 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Mon, 5 Oct 2026 23:59:38 -0700 Subject: [PATCH 09/14] empty commit to trigger ci From 048d6b68ad17d70c935d4cd150c4e06772b44191 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 6 Oct 2026 11:53:14 -0700 Subject: [PATCH 10/14] docfx optimization attempt --- .github/workflows/docs.yml | 1 + .../docfx_yaml/extension.py | 176 +++++++++++++++--- 2 files changed, 149 insertions(+), 28 deletions(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 5e70060d2832..d2318ce96aed 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -171,6 +171,7 @@ jobs: echo "Built local wheel: ${WHEEL_PATH}" echo "gcp-sphinx-docfx-yaml @ file://${WHEEL_PATH}" > /tmp/uv_overrides.txt echo "UV_OVERRIDE=/tmp/uv_overrides.txt" >> $GITHUB_ENV + echo "UV_FIND_LINKS=/tmp/local_wheels" >> $GITHUB_ENV echo "PIP_FIND_LINKS=/tmp/local_wheels" >> $GITHUB_ENV - name: Run docfx for ${{ matrix.package_shard.description }} env: diff --git a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py index a1d6050dac73..2a98949a73ed 100644 --- a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py +++ b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py @@ -1032,6 +1032,58 @@ def _extract_type_name(annotation: Any) -> str: return type_name +class _AllClassesVisitor(ast.NodeVisitor): + """AST visitor that records starting line numbers for all classes in a file.""" + + def __init__(self) -> None: + self.stack: list[str] = [] + self.class_lines: dict[str, int] = {} + + def visit_FunctionDef(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None: + self.stack.append(node.name) + self.stack.append("") + self.generic_visit(node) + self.stack.pop() + self.stack.pop() + + visit_AsyncFunctionDef = visit_FunctionDef # type: ignore[assignment] + + def visit_ClassDef(self, node: ast.ClassDef) -> None: + self.stack.append(node.name) + qualname = ".".join(self.stack) + if qualname not in self.class_lines: + line_number = ( + node.decorator_list[0].lineno if node.decorator_list else node.lineno + ) + self.class_lines[qualname] = line_number + self.generic_visit(node) + self.stack.pop() + + +_CLASS_LINES_CACHE: dict[str, dict[str, int]] = {} + + +def _get_start_line(obj: Any, full_path: str) -> int: + """Returns the starting line number of an object, caching AST parses per file.""" + unwrapped = inspect.unwrap(obj) + if inspect.isclass(unwrapped) and full_path: + class_lines = _CLASS_LINES_CACHE.get(full_path) + if class_lines is None: + try: + with open(full_path, "rb") as f: + tree = ast.parse(f.read()) + visitor = _AllClassesVisitor() + visitor.visit(tree) + class_lines = visitor.class_lines + except Exception: + class_lines = {} + _CLASS_LINES_CACHE[full_path] = class_lines + qualname = getattr(unwrapped, "__qualname__", None) + if qualname and qualname in class_lines: + return class_lines[qualname] + return inspect.getsourcelines(obj)[1] + + def _create_datam( app: sphinx.application.Sphinx, cls: str | None, @@ -1187,7 +1239,7 @@ def _update_friendly_package_name(path): # Make relative path = path.replace(os.sep, "", 1) - start_line = inspect.getsourcelines(obj)[1] + start_line = _get_start_line(obj, full_path) path = _update_friendly_package_name(path) @@ -1920,6 +1972,43 @@ def _render_summary_content( return summary_content +_UID_INDEX_CACHE: dict[Any, tuple[tuple[str, ...], dict[Any, Any]]] = {} + + +def _get_uid_index(known_uids: list[str]) -> tuple[tuple[str, ...], dict[Any, Any]]: + """Builds and caches a prefix tuple and character Trie for fast UID substring matching.""" + if not known_uids: + return ((), {}) + if len(known_uids) < 100: + cache_key: Any = tuple(known_uids) + else: + cache_key = (id(known_uids), len(known_uids), known_uids[0], known_uids[-1]) + cached = _UID_INDEX_CACHE.get(cache_key) + if cached is not None: + return cached + + root_prefixes = tuple({u.split(".")[0] for u in known_uids if u}) + trie: dict[Any, Any] = {} + for rank, uid in enumerate(known_uids): + if not uid: + continue + node = trie + for ch in uid: + nxt = node.get(ch) + if nxt is None: + nxt = {} + node[ch] = nxt + node = nxt + if None not in node: + node[None] = (rank, uid) + + result = (root_prefixes, trie) + if len(_UID_INDEX_CACHE) > 16: + _UID_INDEX_CACHE.clear() + _UID_INDEX_CACHE[cache_key] = result + return result + + def find_uid_to_convert( current_word: str, words: list[str], @@ -1944,7 +2033,31 @@ def find_uid_to_convert( None if current word does not contain any reference `uid`, or the `uid` that should be converted. """ - for uid in known_uids: + root_prefixes, trie = _get_uid_index(known_uids) + if not any(p in current_word for p in root_prefixes): + return None + + matches = [] + n = len(current_word) + for i in range(n): + node = trie.get(current_word[i]) + if node is None: + continue + if None in node: + matches.append(node[None]) + for j in range(i + 1, n): + node = node.get(current_word[j]) + if node is None: + break + if None in node: + matches.append(node[None]) + + if not matches: + return None + if len(matches) > 1: + matches.sort(key=lambda item: item[0]) + + for _, uid in matches: # Do not convert references to itself or containing partial # references. This could result in `storage.types.ReadSession` being # prematurely converted to @@ -1954,24 +2067,23 @@ def find_uid_to_convert( if uid in current_object_name: continue - if uid in current_word: - # If the cross reference has been processed already, " Date: Tue, 6 Oct 2026 13:31:38 -0700 Subject: [PATCH 11/14] speed optimizations --- .../docfx_yaml/extension.py | 50 ++++++++++++-- .../docfx_yaml/markdown_utils.py | 65 ++++++++++++++----- .../gcp-sphinx-docfx-yaml/docfx_yaml/utils.py | 24 ++++--- .../tests/test_helpers.py | 62 ++++++++++++++++++ 4 files changed, 168 insertions(+), 33 deletions(-) diff --git a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py index 2a98949a73ed..29746ba59747 100644 --- a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py +++ b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py @@ -29,7 +29,7 @@ import shutil from collections import defaultdict from collections.abc import Mapping, MutableSet, Sequence -from functools import partial +from functools import lru_cache, partial from itertools import zip_longest from pathlib import Path from typing import Any, Iterable @@ -160,6 +160,9 @@ class Bcolors: summary_type: [[], []] # entry name then entry content for summary_type in set(_SUMMARY_TYPE_BY_ITEM_TYPE.values()) } +_SUMMARY_SEEN_ITEMS: dict[str, set[str]] = { + summary_type: set() for summary_type in set(_SUMMARY_TYPE_BY_ITEM_TYPE.values()) +} # Mapping for each summary page entry's file name and entry name. _FILE_NAME_AND_ENTRY_NAME_BY_SUMMARY_TYPE = { CLASS: ("summary_class.yml", "Classes"), @@ -191,12 +194,43 @@ def _grab_repo_metadata() -> Mapping[str, str] | None: return None +def _optimize_sphinx_pipeline(app: sphinx.application.Sphinx) -> None: + """Disables unused Sphinx work (intersphinx HTTP fetches, viewcode, and HTML rendering).""" + # DocFX YAML generation does not use intersphinx inventories; clearing the + # mapping before intersphinx's builder-inited handler runs avoids external + # HTTP fetches and timeouts on every package build. + if hasattr(app.config, "intersphinx_mapping"): + app.config.intersphinx_mapping = {} + + # Remove sphinx.ext.viewcode listeners so Sphinx does not tokenize and + # highlight all Python source modules into throwaway _modules/*.html pages. + if hasattr(app, "events") and hasattr(app.events, "listeners"): + for event_listeners in app.events.listeners.values(): + event_listeners[:] = [ + listener + for listener in event_listeners + if getattr(listener.handler, "__module__", "") != "sphinx.ext.viewcode" + ] + + # When running with the html builder, DocFX YAML collects all metadata + # during the read phase and writes YAML in build-finished; skip rendering + # throwaway Jinja2 HTML pages and search indices. + if getattr(app.builder, "name", None) == "html": + app.builder.write = lambda *args, **kwargs: None + app.builder.finish = lambda *args, **kwargs: None + + def build_init(app: sphinx.application.Sphinx) -> None: """Initializes the build. Args: app (sphinx.application.Sphinx): The sphinx application. """ + for summary_type in _ENTRY_NAME_AND_ENTRY_CONTENT_BY_SUMMARY_TYPE: + _ENTRY_NAME_AND_ENTRY_CONTENT_BY_SUMMARY_TYPE[summary_type][0].clear() + _ENTRY_NAME_AND_ENTRY_CONTENT_BY_SUMMARY_TYPE[summary_type][1].clear() + _SUMMARY_SEEN_ITEMS[summary_type].clear() + print("Retrieving repository metadata.") if not (repo_metadata := _grab_repo_metadata()): print("Failed to retrieve repository metadata.") @@ -1534,6 +1568,7 @@ def _reformat_pattern(code: str, pattern: str) -> str: return code +@lru_cache(maxsize=4096) def format_code(code: str) -> str: """Reformats code using black.format_str(). @@ -1882,10 +1917,9 @@ def _find_and_add_summary_details( uid = yaml_data.get("uid", "") item_to_add = uid if summary_type == CLASS else f"{uid}-summary" - if ( - item_to_add - not in _ENTRY_NAME_AND_ENTRY_CONTENT_BY_SUMMARY_TYPE[summary_type][0] - ): + seen_items = _SUMMARY_SEEN_ITEMS[summary_type] + if item_to_add not in seen_items: + seen_items.add(item_to_add) _ENTRY_NAME_AND_ENTRY_CONTENT_BY_SUMMARY_TYPE[summary_type][0].append( item_to_add ) @@ -2529,9 +2563,10 @@ def convert_module_to_package_if_needed(obj): if "references" in obj: # Ensure that references have no duplicate ref - ref_uids = [r["uid"] for r in references] + ref_uids = {r["uid"] for r in references} for ref_obj in obj["references"]: if ref_obj["uid"] not in ref_uids: + ref_uids.add(ref_obj["uid"]) references.append(ref_obj) obj.pop("references") @@ -2770,6 +2805,8 @@ def missing_reference( Returns: Any: The new node. """ + if getattr(app.builder, "name", None) == "markdown": + return None reftarget = "" refdoc = "" reftype = "" @@ -2813,6 +2850,7 @@ def setup(app: sphinx.application.Sphinx) -> None: app.add_directive("remarks", RemarksDirective) app.add_directive("todo", TodoDirective) + app.connect("builder-inited", _optimize_sphinx_pipeline, priority=100) app.connect("builder-inited", build_init) app.connect("autodoc-process-docstring", process_docstring) app.connect("autodoc-process-signature", process_signature) diff --git a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/markdown_utils.py b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/markdown_utils.py index 42c018cf77c5..782ef8ba68e6 100644 --- a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/markdown_utils.py +++ b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/markdown_utils.py @@ -357,6 +357,8 @@ def move_markdown_pages( } base_markdown_dir = Path(app.builder.outdir).parent / "markdown" + if not cwd and not base_markdown_dir.exists(): + _generate_markdown_pages(app) markdown_dir = base_markdown_dir.joinpath(*cwd) if cwd else base_markdown_dir @@ -509,27 +511,54 @@ def remove_unused_pages( print(f"Could not delete {page}.") +def _generate_markdown_pages(app: sphinx.application) -> None: + """Renders non-API prose documents to Markdown in-process using the already-read Sphinx environment.""" + cwd = os.getcwd() + if "docs" in cwd: + return + if not getattr(app, "env", None) or not getattr(app.env, "found_docs", None): + return + + from sphinx.util.osutil import ensuredir + from sphinx_markdown_builder.markdown_builder import MarkdownBuilder + + markdown_outdir = str(Path(app.builder.outdir).parent / "markdown") + ensuredir(markdown_outdir) + + known_short_uids = { + uid.split(".")[-1] for uid in getattr(app.env, "docfx_uid_names", ()) + } + docnames = [ + docname + for docname in sorted(app.env.found_docs) + if "/" not in docname + or docname.split("/")[-1].lower() not in known_short_uids + or docname.split("/")[-1].lower() == "index" + ] + + orig_builder = app.builder + md_builder = MarkdownBuilder(app) + md_builder.outdir = markdown_outdir + md_builder.set_environment(app.env) + md_builder.init() + md_builder.prepare_writing(docnames) + app.builder = md_builder + try: + for docname in docnames: + doctree = app.env.get_and_resolve_doctree(docname, md_builder) + md_builder.write_doc_serialized(docname, doctree) + md_builder.write_doc(docname, doctree) + finally: + app.builder = orig_builder + + def run_sphinx_markdown(app: sphinx.application) -> None: """Runs sphinx-build with Markdown builder in the plugin. Args: app (sphinx.application): The sphinx application. """ - cwd = os.getcwd() - relative_srcdir = app.srcdir.removeprefix(f"{cwd}/") - relative_outdir = app.outdir.removeprefix(f"{cwd}/").removesuffix("/html") - # Skip running sphinx-build for Markdown for some unit tests. - # Not required other than to output DocFX YAML. - if "docs" in cwd: - return - - return shell.run( - [ - "sphinx-build", - "-M", - "markdown", - relative_srcdir, - relative_outdir, - ], - hide_output=False, - ) + # Markdown pages are now rendered in-process during build_finished + # (inside move_markdown_pages) reusing the already-read Sphinx environment + # instead of spawning a second sphinx-build subprocess. + return None diff --git a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/utils.py b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/utils.py index 27e35c799311..1ce7c2e9b93e 100644 --- a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/utils.py +++ b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/utils.py @@ -17,12 +17,16 @@ from inspect import signature from docutils import nodes -from docutils.io import StringOutput +from docutils.frontend import OptionParser from docutils.utils import new_document +from sphinx import addnodes from sphinx.application import Sphinx +from .writer import MarkdownTranslator from .writer import MarkdownWriter as Writer +_DEFAULT_SETTINGS = OptionParser(components=(Writer,)).get_default_values() + def slugify(value: str) -> str: """Converts to lowercase, removes non-word characters. @@ -70,14 +74,16 @@ def transform_node(app: Sphinx, node: nodes.Node) -> str: Returns: str: The transformed node as a string. """ - destination = StringOutput(encoding="utf-8") - doc = new_document(b"") + doc = new_document(b"", _DEFAULT_SETTINGS) doc.append(node) - # Resolve refs + # Resolve refs only when the node actually contains pending cross-references doc["docname"] = "inmemory" - app.env.resolve_references(doctree=doc, fromdocname="inmemory", builder=app.builder) - - writer = Writer(app.builder) - writer.write(doc, destination) - return destination.destination.decode("utf-8") + if any(True for _ in node.traverse(addnodes.pending_xref)): + app.env.resolve_references( + doctree=doc, fromdocname="inmemory", builder=app.builder + ) + + visitor = MarkdownTranslator(doc, app.builder) + doc.walkabout(visitor) + return visitor.body diff --git a/packages/gcp-sphinx-docfx-yaml/tests/test_helpers.py b/packages/gcp-sphinx-docfx-yaml/tests/test_helpers.py index 96ac6c72a0a2..5c119b018543 100644 --- a/packages/gcp-sphinx-docfx-yaml/tests/test_helpers.py +++ b/packages/gcp-sphinx-docfx-yaml/tests/test_helpers.py @@ -411,6 +411,68 @@ def test_is_not_valid_python_code(self, invalid_syntax): result = extension.is_valid_python_code(invalid_syntax) self.assertFalse(result) + def test_optimize_sphinx_pipeline(self): + class DummyHandler: + pass + + viewcode_handler = DummyHandler() + viewcode_handler.__module__ = "sphinx.ext.viewcode" + other_handler = DummyHandler() + other_handler.__module__ = "docfx_yaml.extension" + + class DummyListener: + def __init__(self, handler): + self.handler = handler + + class DummyEvents: + def __init__(self): + self.listeners = { + "doctree-read": [ + DummyListener(viewcode_handler), + DummyListener(other_handler), + ] + } + + class DummyConfig: + def __init__(self): + self.intersphinx_mapping = {"python": ("https://example.com", None)} + + class DummyBuilder: + name = "html" + + def write(self, *args, **kwargs): + return "wrote" + + def finish(self, *args, **kwargs): + return "finished" + + class DummyApp: + def __init__(self): + self.config = DummyConfig() + self.events = DummyEvents() + self.builder = DummyBuilder() + + app = DummyApp() + extension._optimize_sphinx_pipeline(app) + + self.assertEqual(app.config.intersphinx_mapping, {}) + self.assertEqual(len(app.events.listeners["doctree-read"]), 1) + self.assertIs( + app.events.listeners["doctree-read"][0].handler, + other_handler, + ) + self.assertIsNone(app.builder.write()) + self.assertIsNone(app.builder.finish()) + + def test_missing_reference_skips_markdown_builder(self): + class DummyBuilder: + name = "markdown" + + class DummyApp: + builder = DummyBuilder() + + self.assertIsNone(extension.missing_reference(DummyApp(), None, None, None)) + if __name__ == "__main__": unittest.main() From 0d753efb90a34c714827a0fc45b37fa85999d89e Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 6 Oct 2026 15:31:02 -0700 Subject: [PATCH 12/14] fixed bigtable issue --- .../docfx_yaml/markdown_utils.py | 11 +---------- packages/gcp-sphinx-docfx-yaml/docfx_yaml/utils.py | 2 ++ 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/markdown_utils.py b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/markdown_utils.py index 782ef8ba68e6..d5d496f1dc45 100644 --- a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/markdown_utils.py +++ b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/markdown_utils.py @@ -525,16 +525,7 @@ def _generate_markdown_pages(app: sphinx.application) -> None: markdown_outdir = str(Path(app.builder.outdir).parent / "markdown") ensuredir(markdown_outdir) - known_short_uids = { - uid.split(".")[-1] for uid in getattr(app.env, "docfx_uid_names", ()) - } - docnames = [ - docname - for docname in sorted(app.env.found_docs) - if "/" not in docname - or docname.split("/")[-1].lower() not in known_short_uids - or docname.split("/")[-1].lower() == "index" - ] + docnames = sorted(app.env.found_docs) orig_builder = app.builder md_builder = MarkdownBuilder(app) diff --git a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/utils.py b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/utils.py index 1ce7c2e9b93e..967957997d38 100644 --- a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/utils.py +++ b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/utils.py @@ -74,6 +74,8 @@ def transform_node(app: Sphinx, node: nodes.Node) -> str: Returns: str: The transformed node as a string. """ + if node.parent is not None: + node = node.deepcopy() doc = new_document(b"", _DEFAULT_SETTINGS) doc.append(node) From 7e30e18d9c3fc6f15278a45050e5980cc5de5c0d Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 6 Oct 2026 16:19:57 -0700 Subject: [PATCH 13/14] cleaned up optimizations --- .../docfx_yaml/extension.py | 41 +++++++++---------- .../tests/test_helpers.py | 35 ++++++---------- 2 files changed, 32 insertions(+), 44 deletions(-) diff --git a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py index 29746ba59747..273369e46d3f 100644 --- a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py +++ b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py @@ -48,6 +48,7 @@ import sphinx.application import yaml from docuploader import shell +from sphinx.builders.html import StandaloneHTMLBuilder from sphinx.errors import ExtensionError from sphinx.ext.napoleon import Config, GoogleDocstring, _process_docstring from sphinx.util import ensuredir @@ -194,30 +195,25 @@ def _grab_repo_metadata() -> Mapping[str, str] | None: return None -def _optimize_sphinx_pipeline(app: sphinx.application.Sphinx) -> None: - """Disables unused Sphinx work (intersphinx HTTP fetches, viewcode, and HTML rendering).""" - # DocFX YAML generation does not use intersphinx inventories; clearing the - # mapping before intersphinx's builder-inited handler runs avoids external - # HTTP fetches and timeouts on every package build. - if hasattr(app.config, "intersphinx_mapping"): - app.config.intersphinx_mapping = {} +class DocFXHTMLBuilder(StandaloneHTMLBuilder): + """HTML builder subclass that skips rendering unused HTML pages during DocFX builds.""" + + def write(self, *args: Any, **kwargs: Any) -> None: + pass + + def finish(self) -> None: + pass + + +def _configure_docfx(app: sphinx.application.Sphinx, config: Any) -> None: + """Configures Sphinx settings and disconnects unused extensions before the build starts.""" + config.intersphinx_mapping = {} - # Remove sphinx.ext.viewcode listeners so Sphinx does not tokenize and - # highlight all Python source modules into throwaway _modules/*.html pages. if hasattr(app, "events") and hasattr(app.events, "listeners"): for event_listeners in app.events.listeners.values(): - event_listeners[:] = [ - listener - for listener in event_listeners - if getattr(listener.handler, "__module__", "") != "sphinx.ext.viewcode" - ] - - # When running with the html builder, DocFX YAML collects all metadata - # during the read phase and writes YAML in build-finished; skip rendering - # throwaway Jinja2 HTML pages and search indices. - if getattr(app.builder, "name", None) == "html": - app.builder.write = lambda *args, **kwargs: None - app.builder.finish = lambda *args, **kwargs: None + for listener in list(event_listeners): + if getattr(listener.handler, "__module__", "") == "sphinx.ext.viewcode": + app.disconnect(listener.id) def build_init(app: sphinx.application.Sphinx) -> None: @@ -2850,7 +2846,8 @@ def setup(app: sphinx.application.Sphinx) -> None: app.add_directive("remarks", RemarksDirective) app.add_directive("todo", TodoDirective) - app.connect("builder-inited", _optimize_sphinx_pipeline, priority=100) + app.add_builder(DocFXHTMLBuilder, override=True) + app.connect("config-inited", _configure_docfx) app.connect("builder-inited", build_init) app.connect("autodoc-process-docstring", process_docstring) app.connect("autodoc-process-signature", process_signature) diff --git a/packages/gcp-sphinx-docfx-yaml/tests/test_helpers.py b/packages/gcp-sphinx-docfx-yaml/tests/test_helpers.py index 5c119b018543..1bcf9a4ed54b 100644 --- a/packages/gcp-sphinx-docfx-yaml/tests/test_helpers.py +++ b/packages/gcp-sphinx-docfx-yaml/tests/test_helpers.py @@ -411,7 +411,7 @@ def test_is_not_valid_python_code(self, invalid_syntax): result = extension.is_valid_python_code(invalid_syntax) self.assertFalse(result) - def test_optimize_sphinx_pipeline(self): + def test_configure_docfx_and_builder(self): class DummyHandler: pass @@ -421,15 +421,16 @@ class DummyHandler: other_handler.__module__ = "docfx_yaml.extension" class DummyListener: - def __init__(self, handler): + def __init__(self, listener_id, handler): + self.id = listener_id self.handler = handler class DummyEvents: def __init__(self): self.listeners = { "doctree-read": [ - DummyListener(viewcode_handler), - DummyListener(other_handler), + DummyListener(1, viewcode_handler), + DummyListener(2, other_handler), ] } @@ -437,32 +438,22 @@ class DummyConfig: def __init__(self): self.intersphinx_mapping = {"python": ("https://example.com", None)} - class DummyBuilder: - name = "html" - - def write(self, *args, **kwargs): - return "wrote" - - def finish(self, *args, **kwargs): - return "finished" - class DummyApp: def __init__(self): self.config = DummyConfig() self.events = DummyEvents() - self.builder = DummyBuilder() + self.disconnected = [] + + def disconnect(self, listener_id): + self.disconnected.append(listener_id) app = DummyApp() - extension._optimize_sphinx_pipeline(app) + extension._configure_docfx(app, app.config) self.assertEqual(app.config.intersphinx_mapping, {}) - self.assertEqual(len(app.events.listeners["doctree-read"]), 1) - self.assertIs( - app.events.listeners["doctree-read"][0].handler, - other_handler, - ) - self.assertIsNone(app.builder.write()) - self.assertIsNone(app.builder.finish()) + self.assertEqual(app.disconnected, [1]) + self.assertIsNone(extension.DocFXHTMLBuilder.write(None)) + self.assertIsNone(extension.DocFXHTMLBuilder.finish(None)) def test_missing_reference_skips_markdown_builder(self): class DummyBuilder: From bbe7bf8011ec25d34c847685d87538ffa3dbfc87 Mon Sep 17 00:00:00 2001 From: Daniel Sanche Date: Tue, 6 Oct 2026 16:37:18 -0700 Subject: [PATCH 14/14] simplified optimizations --- .../docfx_yaml/extension.py | 231 ++++++------------ .../docfx_yaml/markdown_utils.py | 41 ++-- .../tests/test_helpers.py | 57 +---- 3 files changed, 99 insertions(+), 230 deletions(-) diff --git a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py index 273369e46d3f..cf411411c6b8 100644 --- a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py +++ b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py @@ -161,9 +161,6 @@ class Bcolors: summary_type: [[], []] # entry name then entry content for summary_type in set(_SUMMARY_TYPE_BY_ITEM_TYPE.values()) } -_SUMMARY_SEEN_ITEMS: dict[str, set[str]] = { - summary_type: set() for summary_type in set(_SUMMARY_TYPE_BY_ITEM_TYPE.values()) -} # Mapping for each summary page entry's file name and entry name. _FILE_NAME_AND_ENTRY_NAME_BY_SUMMARY_TYPE = { CLASS: ("summary_class.yml", "Classes"), @@ -208,12 +205,10 @@ def finish(self) -> None: def _configure_docfx(app: sphinx.application.Sphinx, config: Any) -> None: """Configures Sphinx settings and disconnects unused extensions before the build starts.""" config.intersphinx_mapping = {} - - if hasattr(app, "events") and hasattr(app.events, "listeners"): - for event_listeners in app.events.listeners.values(): - for listener in list(event_listeners): - if getattr(listener.handler, "__module__", "") == "sphinx.ext.viewcode": - app.disconnect(listener.id) + for listeners in getattr(getattr(app, "events", None), "listeners", {}).values(): + for listener in list(listeners): + if getattr(listener.handler, "__module__", "") == "sphinx.ext.viewcode": + app.disconnect(listener.id) def build_init(app: sphinx.application.Sphinx) -> None: @@ -222,11 +217,6 @@ def build_init(app: sphinx.application.Sphinx) -> None: Args: app (sphinx.application.Sphinx): The sphinx application. """ - for summary_type in _ENTRY_NAME_AND_ENTRY_CONTENT_BY_SUMMARY_TYPE: - _ENTRY_NAME_AND_ENTRY_CONTENT_BY_SUMMARY_TYPE[summary_type][0].clear() - _ENTRY_NAME_AND_ENTRY_CONTENT_BY_SUMMARY_TYPE[summary_type][1].clear() - _SUMMARY_SEEN_ITEMS[summary_type].clear() - print("Retrieving repository metadata.") if not (repo_metadata := _grab_repo_metadata()): print("Failed to retrieve repository metadata.") @@ -234,9 +224,6 @@ def build_init(app: sphinx.application.Sphinx) -> None: else: print("Successfully retrieved repository metadata.") app.env.library_shortname = repo_metadata["name"] - print("Running sphinx-build with Markdown first...") - markdown_utils.run_sphinx_markdown(app) - print("Completed running sphinx-build with Markdown files.") """ Set up environment data @@ -1062,56 +1049,33 @@ def _extract_type_name(annotation: Any) -> str: return type_name -class _AllClassesVisitor(ast.NodeVisitor): - """AST visitor that records starting line numbers for all classes in a file.""" - - def __init__(self) -> None: - self.stack: list[str] = [] - self.class_lines: dict[str, int] = {} - - def visit_FunctionDef(self, node: ast.FunctionDef | ast.AsyncFunctionDef) -> None: - self.stack.append(node.name) - self.stack.append("") - self.generic_visit(node) - self.stack.pop() - self.stack.pop() - - visit_AsyncFunctionDef = visit_FunctionDef # type: ignore[assignment] - - def visit_ClassDef(self, node: ast.ClassDef) -> None: - self.stack.append(node.name) - qualname = ".".join(self.stack) - if qualname not in self.class_lines: - line_number = ( - node.decorator_list[0].lineno if node.decorator_list else node.lineno - ) - self.class_lines[qualname] = line_number - self.generic_visit(node) - self.stack.pop() - - -_CLASS_LINES_CACHE: dict[str, dict[str, int]] = {} - +@lru_cache(maxsize=512) +def _get_class_lines(full_path: str) -> dict[str, int]: + """Parses a file once and maps class qualnames to their starting line numbers.""" + lines: dict[str, int] = {} + + def _visit(node: ast.AST, prefix: str = "") -> None: + for child in ast.iter_child_nodes(node): + if isinstance(child, ast.ClassDef): + qual = f"{prefix}{child.name}" + lines.setdefault( + qual, + child.decorator_list[0].lineno + if child.decorator_list + else child.lineno, + ) + _visit(child, f"{qual}.") + elif isinstance(child, (ast.FunctionDef, ast.AsyncFunctionDef)): + _visit(child, f"{prefix}{child.name}..") + else: + _visit(child, prefix) -def _get_start_line(obj: Any, full_path: str) -> int: - """Returns the starting line number of an object, caching AST parses per file.""" - unwrapped = inspect.unwrap(obj) - if inspect.isclass(unwrapped) and full_path: - class_lines = _CLASS_LINES_CACHE.get(full_path) - if class_lines is None: - try: - with open(full_path, "rb") as f: - tree = ast.parse(f.read()) - visitor = _AllClassesVisitor() - visitor.visit(tree) - class_lines = visitor.class_lines - except Exception: - class_lines = {} - _CLASS_LINES_CACHE[full_path] = class_lines - qualname = getattr(unwrapped, "__qualname__", None) - if qualname and qualname in class_lines: - return class_lines[qualname] - return inspect.getsourcelines(obj)[1] + try: + with open(full_path, "rb") as f: + _visit(ast.parse(f.read())) + except Exception: + pass + return lines def _create_datam( @@ -1269,7 +1233,12 @@ def _update_friendly_package_name(path): # Make relative path = path.replace(os.sep, "", 1) - start_line = _get_start_line(obj, full_path) + unwrapped = inspect.unwrap(obj) + start_line = ( + _get_class_lines(full_path).get(getattr(unwrapped, "__qualname__", ""), 0) + if inspect.isclass(unwrapped) + else 0 + ) or inspect.getsourcelines(obj)[1] path = _update_friendly_package_name(path) @@ -1913,9 +1882,10 @@ def _find_and_add_summary_details( uid = yaml_data.get("uid", "") item_to_add = uid if summary_type == CLASS else f"{uid}-summary" - seen_items = _SUMMARY_SEEN_ITEMS[summary_type] - if item_to_add not in seen_items: - seen_items.add(item_to_add) + if ( + item_to_add + not in _ENTRY_NAME_AND_ENTRY_CONTENT_BY_SUMMARY_TYPE[summary_type][0] + ): _ENTRY_NAME_AND_ENTRY_CONTENT_BY_SUMMARY_TYPE[summary_type][0].append( item_to_add ) @@ -2002,43 +1972,6 @@ def _render_summary_content( return summary_content -_UID_INDEX_CACHE: dict[Any, tuple[tuple[str, ...], dict[Any, Any]]] = {} - - -def _get_uid_index(known_uids: list[str]) -> tuple[tuple[str, ...], dict[Any, Any]]: - """Builds and caches a prefix tuple and character Trie for fast UID substring matching.""" - if not known_uids: - return ((), {}) - if len(known_uids) < 100: - cache_key: Any = tuple(known_uids) - else: - cache_key = (id(known_uids), len(known_uids), known_uids[0], known_uids[-1]) - cached = _UID_INDEX_CACHE.get(cache_key) - if cached is not None: - return cached - - root_prefixes = tuple({u.split(".")[0] for u in known_uids if u}) - trie: dict[Any, Any] = {} - for rank, uid in enumerate(known_uids): - if not uid: - continue - node = trie - for ch in uid: - nxt = node.get(ch) - if nxt is None: - nxt = {} - node[ch] = nxt - node = nxt - if None not in node: - node[None] = (rank, uid) - - result = (root_prefixes, trie) - if len(_UID_INDEX_CACHE) > 16: - _UID_INDEX_CACHE.clear() - _UID_INDEX_CACHE[cache_key] = result - return result - - def find_uid_to_convert( current_word: str, words: list[str], @@ -2063,31 +1996,9 @@ def find_uid_to_convert( None if current word does not contain any reference `uid`, or the `uid` that should be converted. """ - root_prefixes, trie = _get_uid_index(known_uids) - if not any(p in current_word for p in root_prefixes): + if "." not in current_word: return None - - matches = [] - n = len(current_word) - for i in range(n): - node = trie.get(current_word[i]) - if node is None: - continue - if None in node: - matches.append(node[None]) - for j in range(i + 1, n): - node = node.get(current_word[j]) - if node is None: - break - if None in node: - matches.append(node[None]) - - if not matches: - return None - if len(matches) > 1: - matches.sort(key=lambda item: item[0]) - - for _, uid in matches: + for uid in known_uids: # Do not convert references to itself or containing partial # references. This could result in `storage.types.ReadSession` being # prematurely converted to @@ -2097,23 +2008,24 @@ def find_uid_to_convert( if uid in current_object_name: continue - # If the cross reference has been processed already, " 50: return content - example_text = "Examples:" - words = content.split(" ") - - # Contains a list of words that is not a valid reference or converted - # references. - processed_words = [] - # Used to keep track of current position to avoid converting if needed. example_index = len(content) for index, word in enumerate(words): @@ -2423,6 +2332,7 @@ def convert_module_to_package_if_needed(obj): ensuredir(normalized_outdir) # Add markdown pages to the configured output directory. + markdown_utils.run_sphinx_markdown(app) markdown_utils.move_markdown_pages(app, normalized_outdir) pkg_toc_yaml = [] @@ -2559,10 +2469,9 @@ def convert_module_to_package_if_needed(obj): if "references" in obj: # Ensure that references have no duplicate ref - ref_uids = {r["uid"] for r in references} + ref_uids = [r["uid"] for r in references] for ref_obj in obj["references"]: if ref_obj["uid"] not in ref_uids: - ref_uids.add(ref_obj["uid"]) references.append(ref_obj) obj.pop("references") diff --git a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/markdown_utils.py b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/markdown_utils.py index d5d496f1dc45..5028eac50e22 100644 --- a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/markdown_utils.py +++ b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/markdown_utils.py @@ -357,8 +357,6 @@ def move_markdown_pages( } base_markdown_dir = Path(app.builder.outdir).parent / "markdown" - if not cwd and not base_markdown_dir.exists(): - _generate_markdown_pages(app) markdown_dir = base_markdown_dir.joinpath(*cwd) if cwd else base_markdown_dir @@ -511,25 +509,30 @@ def remove_unused_pages( print(f"Could not delete {page}.") -def _generate_markdown_pages(app: sphinx.application) -> None: - """Renders non-API prose documents to Markdown in-process using the already-read Sphinx environment.""" - cwd = os.getcwd() - if "docs" in cwd: - return - if not getattr(app, "env", None) or not getattr(app.env, "found_docs", None): +def run_sphinx_markdown(app: sphinx.application) -> None: + """Runs Markdown builder in-process reusing the already-read Sphinx environment. + + Args: + app (sphinx.application): The sphinx application. + """ + # Skip running Markdown builder for some unit tests. + # Not required other than to output DocFX YAML. + markdown_outdir = Path(app.builder.outdir).parent / "markdown" + if ( + "docs" in os.getcwd() + or markdown_outdir.exists() + or not getattr(app.env, "found_docs", None) + ): return from sphinx.util.osutil import ensuredir from sphinx_markdown_builder.markdown_builder import MarkdownBuilder - markdown_outdir = str(Path(app.builder.outdir).parent / "markdown") - ensuredir(markdown_outdir) - + ensuredir(str(markdown_outdir)) docnames = sorted(app.env.found_docs) - orig_builder = app.builder md_builder = MarkdownBuilder(app) - md_builder.outdir = markdown_outdir + md_builder.outdir = str(markdown_outdir) md_builder.set_environment(app.env) md_builder.init() md_builder.prepare_writing(docnames) @@ -541,15 +544,3 @@ def _generate_markdown_pages(app: sphinx.application) -> None: md_builder.write_doc(docname, doctree) finally: app.builder = orig_builder - - -def run_sphinx_markdown(app: sphinx.application) -> None: - """Runs sphinx-build with Markdown builder in the plugin. - - Args: - app (sphinx.application): The sphinx application. - """ - # Markdown pages are now rendered in-process during build_finished - # (inside move_markdown_pages) reusing the already-read Sphinx environment - # instead of spawning a second sphinx-build subprocess. - return None diff --git a/packages/gcp-sphinx-docfx-yaml/tests/test_helpers.py b/packages/gcp-sphinx-docfx-yaml/tests/test_helpers.py index 1bcf9a4ed54b..6ddddfdd0d9e 100644 --- a/packages/gcp-sphinx-docfx-yaml/tests/test_helpers.py +++ b/packages/gcp-sphinx-docfx-yaml/tests/test_helpers.py @@ -1,5 +1,6 @@ import tempfile import unittest +import unittest.mock from parameterized import parameterized from yaml import Loader, load @@ -412,57 +413,25 @@ def test_is_not_valid_python_code(self, invalid_syntax): self.assertFalse(result) def test_configure_docfx_and_builder(self): - class DummyHandler: - pass - - viewcode_handler = DummyHandler() - viewcode_handler.__module__ = "sphinx.ext.viewcode" - other_handler = DummyHandler() - other_handler.__module__ = "docfx_yaml.extension" - - class DummyListener: - def __init__(self, listener_id, handler): - self.id = listener_id - self.handler = handler - - class DummyEvents: - def __init__(self): - self.listeners = { - "doctree-read": [ - DummyListener(1, viewcode_handler), - DummyListener(2, other_handler), - ] - } - - class DummyConfig: - def __init__(self): - self.intersphinx_mapping = {"python": ("https://example.com", None)} - - class DummyApp: - def __init__(self): - self.config = DummyConfig() - self.events = DummyEvents() - self.disconnected = [] - - def disconnect(self, listener_id): - self.disconnected.append(listener_id) - - app = DummyApp() + app = unittest.mock.MagicMock() + app.config.intersphinx_mapping = {"python": ("https://example.com", None)} + viewcode_listener = unittest.mock.MagicMock(id=1) + viewcode_listener.handler.__module__ = "sphinx.ext.viewcode" + other_listener = unittest.mock.MagicMock(id=2) + other_listener.handler.__module__ = "docfx_yaml.extension" + app.events.listeners = {"doctree-read": [viewcode_listener, other_listener]} + extension._configure_docfx(app, app.config) self.assertEqual(app.config.intersphinx_mapping, {}) - self.assertEqual(app.disconnected, [1]) + app.disconnect.assert_called_once_with(1) self.assertIsNone(extension.DocFXHTMLBuilder.write(None)) self.assertIsNone(extension.DocFXHTMLBuilder.finish(None)) def test_missing_reference_skips_markdown_builder(self): - class DummyBuilder: - name = "markdown" - - class DummyApp: - builder = DummyBuilder() - - self.assertIsNone(extension.missing_reference(DummyApp(), None, None, None)) + app = unittest.mock.MagicMock() + app.builder.name = "markdown" + self.assertIsNone(extension.missing_reference(app, None, None, None)) if __name__ == "__main__":