shahar1 commented on code in PR #75001: URL: https://github.com/apache/airflow/pull/75001#discussion_r4345092956
########## scripts/ci/docker-compose/mount-sphinx-airflow-theme.yml: ########## @@ -0,0 +1,26 @@ +# 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. +--- +services: + airflow: + # Added by `breeze build-docs --sphinx-airflow-theme-path`. The mount is read-only so the + # in-container install cannot leave root-owned build output in the airflow-site checkout. + volumes: + - type: bind + source: ${SPHINX_AIRFLOW_THEME_PATH} + target: /opt/sphinx_airflow_theme + read_only: true Review Comment: Thanks for checking it in the CI image. The mount stays read-only. --- Drafted-by: Claude Code (Opus 5.5) (no human review before posting) ########## scripts/in_container/run_docs_build.sh: ########## @@ -21,6 +21,12 @@ git config --global --add safe.directory /opt/airflow cd "${AIRFLOW_SOURCES}" || exit 1 + +# Mounted by `breeze build-docs --sphinx-airflow-theme-path`. +if [[ -d /opt/sphinx_airflow_theme ]]; then + uv pip install --no-deps --reinstall /opt/sphinx_airflow_theme Review Comment: Done in e721ac97e3: the script now runs `uv pip install --reinstall-package sphinx-airflow-theme /opt/sphinx_airflow_theme`. --- Drafted-by: Claude Code (Opus 5.5) (no human review before posting) ########## dev/breeze/tests/test_developer_commands.py: ########## @@ -109,6 +109,53 @@ def test_python_option_is_rejected(self, runner): assert "no such option" in result.output.lower() +class TestBuildDocsSphinxAirflowTheme: + @pytest.fixture(autouse=True) + def _isolated_env(self, monkeypatch, tmp_path): + monkeypatch.setenv("SKIP_SAVING_CHOICES", "true") + for name in ("BREEZE_SDK", "DISTRIBUTIONS_LIST"): + monkeypatch.delenv(name, raising=False) + # The real _build_python_docs deletes generated/**/apis under AIRFLOW_ROOT_PATH. + monkeypatch.setattr("airflow_breeze.commands.developer_commands.AIRFLOW_ROOT_PATH", tmp_path) + + @pytest.fixture + def docker_mocks(self): + with patch.multiple( + "airflow_breeze.commands.developer_commands", + perform_environment_checks=DEFAULT, + fix_ownership_using_docker=DEFAULT, + cleanup_python_generated_files=DEFAULT, + build_ci_image_if_needed=DEFAULT, + execute_command_in_shell=DEFAULT, + autospec=True, + ) as mocks: + mocks["execute_command_in_shell"].return_value.returncode = 0 + yield mocks + + @pytest.mark.parametrize("pass_option", [True, False]) + def test_theme_path_is_passed_to_shell_params( + self, runner, docker_mocks, tmp_path, monkeypatch, pass_option + ): + (tmp_path / "theme" / "sphinx_airflow_theme" / "static" / "_gen").mkdir(parents=True) + monkeypatch.chdir(tmp_path) + + runner.invoke( + build_docs, + ["--sphinx-airflow-theme-path", "theme"] if pass_option else [], + catch_exceptions=False, + ) + + shell_params = docker_mocks["execute_command_in_shell"].call_args.args[0] + assert shell_params.sphinx_airflow_theme_path == (str(tmp_path / "theme") if pass_option else "") + + def test_theme_without_generated_assets_is_rejected(self, runner, docker_mocks, tmp_path): Review Comment: Done: the test now creates `sphinx_airflow_theme/` without `static/_gen` and also checks the error text, so it fails when only the package folder is checked. --- Drafted-by: Claude Code (Opus 5.5) (no human review before posting) ########## dev/breeze/src/airflow_breeze/commands/developer_commands.py: ########## @@ -1052,11 +1060,21 @@ def build_docs( distributions_list: str, sdk: tuple[str, ...], sdk_docs_only: bool, + sphinx_airflow_theme_path: str | None, doc_packages: tuple[str, ...], ): """ Build documents. """ + if ( Review Comment: Done: `--sphinx-airflow-theme-path` together with `--sdk-docs-only` is now rejected with a usage error. --- Drafted-by: Claude Code (Opus 5.5) (no human review before posting) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
