diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index e8fe1a06f6f2..d2318ce96aed 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,9 +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 + 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 @@ -156,12 +163,81 @@ jobs: run: | python -m pip install --upgrade setuptools pip wheel python -m pip install nox uv - - name: Run docfx + - 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 "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: - BUILD_TYPE: presubmit + 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 + 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 != 'push' + runs-on: ubuntu-latest + permissions: + contents: read + actions: read + 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: | + 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 diff --git a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/extension.py index 1d8479df1b6b..cf411411c6b8 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 @@ -46,13 +46,21 @@ import subprocess 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 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 @@ -184,6 +192,25 @@ def _grab_repo_metadata() -> Mapping[str, str] | None: return None +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 = {} + 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: """Initializes the build. @@ -197,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 @@ -1025,6 +1049,35 @@ def _extract_type_name(annotation: Any) -> str: return type_name +@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) + + try: + with open(full_path, "rb") as f: + _visit(ast.parse(f.read())) + except Exception: + pass + return lines + + def _create_datam( app: sphinx.application.Sphinx, cls: str | None, @@ -1180,7 +1233,12 @@ def _update_friendly_package_name(path): # Make relative path = path.replace(os.sep, "", 1) - start_line = inspect.getsourcelines(obj)[1] + 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) @@ -1475,6 +1533,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(). @@ -1937,6 +1996,8 @@ def find_uid_to_convert( None if current word does not contain any reference `uid`, or the `uid` that should be converted. """ + if "." not in current_word: + return None for uid in known_uids: # Do not convert references to itself or containing partial # references. This could result in `storage.types.ReadSession` being @@ -2014,7 +2075,11 @@ def convert_cross_references( "google.iam.v1.iam_policy_pb2.TestIamPermissionsResponse": iam_policy_link + "#L120-L131", } - known_uids.extend(hard_coded_references.keys()) + for ref_key in hard_coded_references: + if ref_key not in known_uids[-4:]: + known_uids.append(ref_key) + if "google." not in content and len(known_uids) > 50: + return content # Used to keep track of current position to avoid converting if needed. example_index = len(content) @@ -2267,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 = [] @@ -2277,6 +2343,8 @@ def convert_module_to_package_if_needed(obj): # Used to disambiguate entry names yaml_map = {} + known_uids = sorted(app.env.docfx_uid_names.keys(), reverse=True) + # Order matters here, we need modules before lower level classes, # so that we can make sure to inject the TOC properly for data_set in ( @@ -2433,7 +2501,6 @@ def convert_module_to_package_if_needed(obj): # google.cloud.aiplatform.AutoMLForecastingTrainingJob current_object_name = obj["fullName"] - known_uids = sorted(app.env.docfx_uid_names.keys(), reverse=True) # Currently we only need to look in summary, syntax and # attributes for cross references. search_cross_references(obj, current_object_name, known_uids) @@ -2643,6 +2710,8 @@ def missing_reference( Returns: Any: The new node. """ + if getattr(app.builder, "name", None) == "markdown": + return None reftarget = "" refdoc = "" reftype = "" @@ -2686,6 +2755,8 @@ def setup(app: sphinx.application.Sphinx) -> None: app.add_directive("remarks", RemarksDirective) app.add_directive("todo", TodoDirective) + 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/docfx_yaml/markdown_utils.py b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/markdown_utils.py index 42c018cf77c5..5028eac50e22 100644 --- a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/markdown_utils.py +++ b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/markdown_utils.py @@ -510,26 +510,37 @@ def remove_unused_pages( def run_sphinx_markdown(app: sphinx.application) -> None: - """Runs sphinx-build with Markdown builder in the plugin. + """Runs Markdown builder in-process reusing the already-read Sphinx environment. 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. + # Skip running Markdown builder for some unit tests. # Not required other than to output DocFX YAML. - if "docs" in cwd: + 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 - return shell.run( - [ - "sphinx-build", - "-M", - "markdown", - relative_srcdir, - relative_outdir, - ], - hide_output=False, - ) + from sphinx.util.osutil import ensuredir + from sphinx_markdown_builder.markdown_builder import MarkdownBuilder + + ensuredir(str(markdown_outdir)) + docnames = sorted(app.env.found_docs) + orig_builder = app.builder + md_builder = MarkdownBuilder(app) + md_builder.outdir = str(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 diff --git a/packages/gcp-sphinx-docfx-yaml/docfx_yaml/utils.py b/packages/gcp-sphinx-docfx-yaml/docfx_yaml/utils.py index 27e35c799311..967957997d38 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,18 @@ 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"") + if node.parent is not None: + node = node.deepcopy() + 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..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 @@ -411,6 +412,27 @@ def test_is_not_valid_python_code(self, invalid_syntax): result = extension.is_valid_python_code(invalid_syntax) self.assertFalse(result) + def test_configure_docfx_and_builder(self): + 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, {}) + 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): + app = unittest.mock.MagicMock() + app.builder.name = "markdown" + self.assertIsNone(extension.missing_reference(app, None, None, None)) + if __name__ == "__main__": unittest.main()