You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by pi...@apache.org on 2026/07/29 10:13:04 UTC

(airflow) branch main updated: Run UI compile checks in CI when only an OpenAPI spec changes (#70643)

This is an automated email from the ASF dual-hosted git repository.

pierrejeambrun pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git


The following commit(s) were added to refs/heads/main by this push:
     new b43a87e58fc Run UI compile checks in CI when only an OpenAPI spec changes (#70643)
b43a87e58fc is described below

commit b43a87e58fc45a06988d6dd09b1d1e751a6aac7b
Author: Shahar Epstein <60...@users.noreply.github.com>
AuthorDate: Wed Jul 29 13:12:55 2026 +0300

    Run UI compile checks in CI when only an OpenAPI spec changes (#70643)
    
    #68919 changed _private_ui.yaml without regenerating the TypeScript client
    and CI stayed green: selective checks decide whether to skip the
    ts-compile-lint-* hooks from file groups that do not cover the UI codegen
    input specs, while the hooks themselves trigger on any yaml under the
    openapi directories. The stale client then masked a type error that broke
    main for every PR running full static checks. The same gap applied to
    v2-simple-auth-manager-generated.yaml, so a spec-only change also skipped
    the simple auth manager UI hook. Track the codegen-input specs in their own
    file group so spec-only changes run the UI hooks without forcing the full
    test matrix, and drop the main UI hook trigger for the simple auth manager
    spec: that pattern was copied in when the hook was created (#51725), a
    month after the spec was renamed v1 to v2 (#50705), so it has never matched
    a file - #53636 fixed the sibling hook's copy but missed this one. The main
    UI codegen does not consume that spec either.
---
 airflow-core/.pre-commit-config.yaml               |  3 +-
 dev/breeze/doc/ci/04_selective_checks.md           | 14 +++--
 .../src/airflow_breeze/utils/selective_checks.py   | 16 +++++-
 dev/breeze/tests/test_selective_checks.py          | 60 ++++++++++++++++++++++
 4 files changed, 87 insertions(+), 6 deletions(-)

diff --git a/airflow-core/.pre-commit-config.yaml b/airflow-core/.pre-commit-config.yaml
index 591b27fd021..08382f3934f 100644
--- a/airflow-core/.pre-commit-config.yaml
+++ b/airflow-core/.pre-commit-config.yaml
@@ -205,8 +205,7 @@ repos:
         files: |
           (?x)
           ^src/airflow/ui/.*\.(js|ts|tsx|yaml|css|json)$|
-          ^src/airflow/api_fastapi/core_api/openapi/.*\.yaml$|
-          ^src/airflow/api_fastapi/auth/managers/simple/openapi/v1.*\.yaml$
+          ^src/airflow/api_fastapi/core_api/openapi/.*\.yaml$
         exclude: |
           (?x)
           ^src/airflow/ui/node-modules/.*|
diff --git a/dev/breeze/doc/ci/04_selective_checks.md b/dev/breeze/doc/ci/04_selective_checks.md
index f8e27582ee9..3d64275757c 100644
--- a/dev/breeze/doc/ci/04_selective_checks.md
+++ b/dev/breeze/doc/ci/04_selective_checks.md
@@ -252,6 +252,7 @@ representative examples (file → effect):
 | `.github/workflows/codeql-analysis.yml` (non-test workflow) | **basic checks only**                                           | non-test workflow → cannot affect tests (env-files carve-out) |
 | `scripts/ci/prek/check_*.py` (static-check hook)       | CI image + static checks, **no full matrix**                         | prek hooks are static checks → `Prek files` carve-out |
 | the generated OpenAPI spec                             | **full matrix**                                                      | the API *contract* ripples to UI codegen + every client |
+| `core_api/openapi/_private_ui.yaml` (UI-only spec)     | UI compile/lint prek hooks, **no full matrix**                       | matches `UI OpenAPI files`; the UI codegen input must be type-checked, but the public contract is unchanged |
 | `chart/templates/...yaml` (on `main`)                  | `run_helm_tests` (+ PROD image)                                      | matches `HELM_FILES`; Helm tests only on `main` |
 | `task-sdk/.../task_runner.py` or `airflow-core/tests/integration/otel/...` | the `otel` core integration                       | matches `OTEL_FILES`; the otel integration tests assert the span hierarchy task_runner emits |
 | `airflow-core/src/airflow/ui/...tsx` only              | `run_ui_tests`, **no** unit tests                                    | "only new-UI files" short-circuit skips Python unit tests |
@@ -363,6 +364,11 @@ We have the following Groups of files for CI that determine which tests are run:
 * `Always test files` - Files that belong to "Always" run tests.
 * `API tests files` and `Codegen test files` - those are OpenAPI definition files that impact
   Open API specification and determine that we should run dedicated API tests.
+* `UI OpenAPI files` - the OpenAPI spec yamls under `core_api/openapi/` and under the simple
+  auth manager's `openapi/` that are the inputs of the UI client codegen. Membership in this
+  group does not force full tests (a generated spec still does, via `Codegen test files`); it
+  only keeps the UI compile/lint prek hooks from being skipped
+  (see [Skipping prek hooks](#skipping-prek-hooks-static-checks)).
 * `Helm files` - change in those files impacts helm "rendering" tests - `chart` folder (which contains the chart sources and tests under `chart/tests/`).
 * `Build files` - change in the files indicates that we should run  `upgrade to newer dependencies` -
   build dependencies in `pyproject.toml` and  generated dependencies files in `generated` folder.
@@ -372,7 +378,6 @@ We have the following Groups of files for CI that determine which tests are run:
 * `DOC files` - change in those files indicate that we should run documentation builds (both airflow sources
   and airflow documentation)
 * `UI files` - those are files for the new full React UI (useful to determine if UI tests should run)
-* `WWW files` - those are files for the WWW part of our UI (useful to determine if UI tests should run)
 * `System test files` - those are the files that are part of system tests (system tests are not automatically
   run in our CI, but Airflow stakeholders are running the tests and expose dashboards for them at
   [System Test Dashbards](https://airflow.apache.org/ecosystem/#airflow-provider-system-test-dashboards)
@@ -497,8 +502,11 @@ when some files are not changed. Those are the rules implemented:
     `mypy-task-sdk-integration-tests`, `mypy-docker-tests`, `mypy-kubernetes-tests`)
   * for each `shared/<dist>` workspace member, `mypy-shared-<dist>` is skipped when no
     file under `shared/<dist>/` changed (enumerated at runtime)
-  * if no `UI files` changed - `ts-compile-format-lint-ui` check is skipped
-  * if no `WWW files` changed - `ts-compile-format-lint-www` check is skipped
+  * if neither `UI files` nor `UI OpenAPI files` changed - the `ts-compile-lint-ui` and
+    `ts-compile-lint-simple-auth-manager-ui` checks are skipped. The `UI OpenAPI files` group
+    covers the union of those hooks' own openapi `files:` triggers, so a spec-only change
+    (e.g. `_private_ui.yaml`) still runs them - otherwise a stale committed client can mask
+    type errors (see #68919)
   * if no `All Python files` changed - `flynt` check is skipped
   * if no `Helm files` changed - `lint-helm-chart` check is skipped
   * if no `Java SDK files` changed - `ktlint` check is skipped (it runs the java-sdk Gradle
diff --git a/dev/breeze/src/airflow_breeze/utils/selective_checks.py b/dev/breeze/src/airflow_breeze/utils/selective_checks.py
index 95fe83b0e38..63ac975ea66 100644
--- a/dev/breeze/src/airflow_breeze/utils/selective_checks.py
+++ b/dev/breeze/src/airflow_breeze/utils/selective_checks.py
@@ -117,6 +117,7 @@ class FileGroupForCi(Enum):
     DOC_FILES = auto()
     TEXT_NON_DOC_FILES = auto()
     UI_FILES = auto()
+    UI_OPENAPI_FILES = auto()
     SYSTEM_TEST_FILES = auto()
     KUBERNETES_FILES = auto()
     TASK_SDK_FILES = auto()
@@ -379,6 +380,17 @@ CI_FILE_GROUP_MATCHES: HashableDict[FileGroupForCi] = HashableDict(
             r"^airflow-core/src/airflow/ui/",
             r"^airflow-core/src/airflow/api_fastapi/auth/managers/simple/ui/",
         ],
+        # The OpenAPI spec yamls that are inputs of the UI client codegen. Must cover the UNION of
+        # the openapi `files:` triggers of `ts-compile-lint-ui` and
+        # `ts-compile-lint-simple-auth-manager-ui` in `airflow-core/.pre-commit-config.yaml` —
+        # selective checks skip the two hooks as one unit, so this group is a strict superset of
+        # the first hook's triggers; do not "re-sync" it down to a single hook. A spec-only change
+        # (e.g. `_private_ui.yaml`) must not skip those hooks, otherwise a stale committed client
+        # masks type errors in CI (https://github.com/apache/airflow/pull/68919).
+        FileGroupForCi.UI_OPENAPI_FILES: [
+            r"^airflow-core/src/airflow/api_fastapi/core_api/openapi/.*\.yaml",
+            r"^airflow-core/src/airflow/api_fastapi/auth/managers/simple/openapi/.*\.yaml",
+        ],
         FileGroupForCi.KUBERNETES_FILES: [
             r"^chart",
             r"^kubernetes-tests",
@@ -1671,7 +1683,9 @@ class SelectiveChecks:
             return ",".join(sorted(prek_hooks_to_skip))
         if not (
             self._matching_files(FileGroupForCi.UI_FILES, CI_FILE_GROUP_MATCHES)
-            or self._matching_files(FileGroupForCi.API_CODEGEN_FILES, CI_FILE_GROUP_MATCHES)
+            # An API_CODEGEN_FILES disjunct would be unreachable here — matching that group
+            # forces full_tests_needed, and skip_prek_hooks returns early above in that case.
+            or self._matching_files(FileGroupForCi.UI_OPENAPI_FILES, CI_FILE_GROUP_MATCHES)
         ):
             prek_hooks_to_skip.add("ts-compile-lint-ui")
             prek_hooks_to_skip.add("ts-compile-lint-simple-auth-manager-ui")
diff --git a/dev/breeze/tests/test_selective_checks.py b/dev/breeze/tests/test_selective_checks.py
index 1dac0e3dd83..699111954d0 100644
--- a/dev/breeze/tests/test_selective_checks.py
+++ b/dev/breeze/tests/test_selective_checks.py
@@ -115,6 +115,18 @@ ALL_SKIPPED_COMMITS_ON_NO_CI_IMAGE = (
 
 ALL_SKIPPED_COMMITS_BY_DEFAULT_ON_ALL_TESTS_NEEDED = "identity,update-uv-lock"
 
+ALL_SKIPPED_COMMITS_IF_ONLY_UI_OPENAPI_CHANGED = (
+    "check-provider-yaml-valid,check-ts-sdk-supervisor-schema,flynt,identity,ktlint,"
+    "lint-helm-chart,mypy-airflow-core,mypy-airflow-ctl,mypy-airflow-ctl-tests,"
+    "mypy-airflow-e2e-tests,mypy-dev,mypy-devel-common,mypy-docker-tests,mypy-helm-tests,"
+    "mypy-kubernetes-tests,mypy-scripts,mypy-shared-configuration,mypy-shared-dagnode,"
+    "mypy-shared-listeners,mypy-shared-logging,mypy-shared-module_loading,"
+    "mypy-shared-observability,mypy-shared-plugins_manager,mypy-shared-providers_discovery,"
+    "mypy-shared-secrets_backend,mypy-shared-secrets_masker,mypy-shared-serialization,"
+    "mypy-shared-state,mypy-shared-template_rendering,mypy-shared-timezones,mypy-task-sdk,"
+    "mypy-task-sdk-integration-tests,update-uv-lock"
+)
+
 ALL_SKIPPED_COMMITS_IF_NO_UI = (
     "check-ts-sdk-supervisor-schema,identity,ktlint,mypy-airflow-core,mypy-airflow-ctl,mypy-airflow-ctl-tests,mypy-airflow-e2e-tests,"
     "mypy-dev,mypy-devel-common,mypy-docker-tests,mypy-helm-tests,mypy-kubernetes-tests,"
@@ -2045,6 +2057,33 @@ def test_provider_yaml_check_not_skipped_when_check_scripts_change(files: tuple[
     assert "check-provider-yaml-valid" not in skip_prek_hooks.split(",")
 
 
+@pytest.mark.parametrize(
+    "files",
+    [
+        pytest.param(
+            ("airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml",),
+            id="private UI spec changed",
+        ),
+        pytest.param(
+            (
+                "airflow-core/src/airflow/api_fastapi/auth/managers/simple/openapi/v2-simple-auth-manager-generated.yaml",
+            ),
+            id="simple auth manager spec changed",
+        ),
+    ],
+)
+def test_ui_compile_hooks_not_skipped_when_ui_openapi_spec_changes(files: tuple[str, ...]):
+    stderr = SelectiveChecks(
+        files=files,
+        github_event=GithubEvents.PULL_REQUEST,
+        commit_ref=NEUTRAL_COMMIT,
+        default_branch="main",
+    )
+    skip_prek_hooks = str(stderr).split("skip-prek-hooks=")[1].split("\n")[0]
+    assert "ts-compile-lint-ui" not in skip_prek_hooks.split(",")
+    assert "ts-compile-lint-simple-auth-manager-ui" not in skip_prek_hooks.split(",")
+
+
 @pytest.mark.parametrize(
     ("files", "expected_outputs"),
     [
@@ -2735,6 +2774,27 @@ def test_expected_output_push(
             },
             id="OpenAPI spec change still forces the full matrix",
         ),
+        pytest.param(
+            ("airflow-core/src/airflow/api_fastapi/core_api/openapi/_private_ui.yaml",),
+            {
+                # Rationale on the UI_OPENAPI_FILES group in selective_checks.py. One param per
+                # spec directory so each pattern of the group is pinned individually - with both
+                # files in one param, dropping either pattern would still pass via the other file.
+                "full-tests-needed": "false",
+                "skip-prek-hooks": ALL_SKIPPED_COMMITS_IF_ONLY_UI_OPENAPI_CHANGED,
+            },
+            id="Private UI OpenAPI spec change runs the UI compile hooks without the full matrix",
+        ),
+        pytest.param(
+            (
+                "airflow-core/src/airflow/api_fastapi/auth/managers/simple/openapi/v2-simple-auth-manager-generated.yaml",
+            ),
+            {
+                "full-tests-needed": "false",
+                "skip-prek-hooks": ALL_SKIPPED_COMMITS_IF_ONLY_UI_OPENAPI_CHANGED,
+            },
+            id="Simple auth manager OpenAPI spec change runs the UI compile hooks",
+        ),
         pytest.param(
             (
                 "airflow-core/src/airflow/assets/",