potiuk commented on code in PR #73241: URL: https://github.com/apache/airflow/pull/73241#discussion_r4077702007
########## dev/breeze/src/airflow_breeze/utils/verification_plan.py: ########## @@ -0,0 +1,159 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import json +import shlex +from dataclasses import asdict, dataclass +from typing import Any + +from airflow_breeze.utils.selective_checks import SelectiveChecks + + +@dataclass(frozen=True) +class VerificationItem: + kind: str + command: str + runs_in: str + + +# SelectiveChecks flag -> (kind, command, runs_in) of the CI job(s) that flag gates. +# Mirrors .github/workflows/ci-amd.yml and basic-tests.yml; "breeze" means the command needs +# Docker and the CI image, "host" means only host tooling. +FLAG_COMMANDS: dict[str, tuple[tuple[str, str, str], ...]] = { Review Comment: The `run_*` classification test catches a new flag, and `run_unit_tests.sh` pins the core/providers rows, but nothing checks the command text of the other entries — if `ci-amd.yml` renames a hook or changes a `--stage`, this table goes stale silently. A cheap guard: for each `prek … --stage manual <hook>` entry, assert `<hook>` appears in `.github/workflows/ci-amd.yml`. ########## dev/breeze/src/airflow_breeze/utils/verification_plan.py: ########## @@ -0,0 +1,159 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import json +import shlex +from dataclasses import asdict, dataclass +from typing import Any + +from airflow_breeze.utils.selective_checks import SelectiveChecks + + +@dataclass(frozen=True) +class VerificationItem: + kind: str + command: str + runs_in: str + + +# SelectiveChecks flag -> (kind, command, runs_in) of the CI job(s) that flag gates. +# Mirrors .github/workflows/ci-amd.yml and basic-tests.yml; "breeze" means the command needs +# Docker and the CI image, "host" means only host tooling. +FLAG_COMMANDS: dict[str, tuple[tuple[str, str, str], ...]] = { + "run_mypy_providers": (("mypy", "prek run --stage manual mypy-providers --all-files", "breeze"),), + "has_migrations": (("schema", "prek run --stage manual migration-round-trip --all-files", "breeze"),), + "run_task_sdk_tests": (("unit", "breeze testing task-sdk-tests", "breeze"),), + "run_airflow_ctl_tests": (("unit", "breeze testing airflow-ctl-tests", "breeze"),), + "run_scripts_tests": (("unit", "uv run --project scripts pytest scripts/tests/", "host"),), + "run_ui_tests": ( + ("ui", "cd airflow-core/src/airflow/ui && pnpm install --frozen-lockfile && pnpm test", "host"), + ( + "ui", + "cd airflow-core/src/airflow/api_fastapi/auth/managers/simple/ui && pnpm install --frozen-lockfile && pnpm test", + "host", + ), + ), + "run_api_codegen": (("schema", "breeze testing python-api-client-tests", "breeze"),), + "run_go_sdk_tests": (("unit", "cd go-sdk && go test ./...", "host"),), + "run_java_sdk_tests": ( + ("unit", "cd java-sdk && ./gradlew test", "host"), + ("docs", "breeze build-docs --sdk-docs-only --sdk=java", "breeze"), + ), + "run_ts_sdk_docs": (("docs", "breeze build-docs --sdk-docs-only --sdk=typescript", "breeze"),), + "run_breeze_integration_tests": ( + ("unit", "cd dev/breeze && uv run --locked pytest", "host"), + ("unit", "cd dev/breeze && uv run --locked pytest -m integration_tests", "host"), + ), +} + +# Gated CI jobs that need a cluster, a prod image or an e2e stack; classified only, never printed. +NOT_RUNNABLE_LOCALLY: frozenset[str] = frozenset( + { + "run_kubernetes_tests", + "run_helm_tests", + "run_kustomize_overlays_tests", + "run_ui_e2e_tests", + "run_task_sdk_integration_tests", + "run_airflow_ctl_integration_tests", + "run_system_tests", + "run_remote_logging_s3_e2e_tests", + "run_remote_logging_elasticsearch_e2e_tests", + "run_remote_logging_opensearch_e2e_tests", + "run_event_driven_e2e_tests", + "run_java_sdk_e2e_tests", + "run_go_sdk_e2e_tests", + "run_openlineage_e2e_tests", + "run_openlineage_e2e_compat_tests", + "run_ts_sdk_e2e_tests", + } +) + + +def build_prek_item(sc: SelectiveChecks, base_ref: str) -> VerificationItem: + if sc.basic_checks_only: + command = f"SKIP_BREEZE_PREK_HOOKS=true SKIP={sc.skip_prek_hooks} prek run --from-ref {shlex.quote(base_ref)} --to-ref HEAD" + return VerificationItem("prek", command, "host") + return VerificationItem("prek", f"SKIP={sc.skip_prek_hooks} prek run --all-files", "breeze") Review Comment: This mirrors CI's static-checks job, but locally `--all-files` is the slowest row by far, and `AGENTS.md` recommends `prek run --from-ref <target> --stage pre-commit` for pre-push checks. Either use the `--from-ref` form here (you already build it for `basic_checks_only`), or add a short comment saying `--all-files` is deliberate because non-basic changes can affect hooks outside the diff. ########## dev/breeze/tests/test_verify_commands.py: ########## @@ -0,0 +1,118 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import json +import re +from subprocess import CompletedProcess +from unittest.mock import patch + +import pytest +from click.testing import CliRunner + +from airflow_breeze.commands.verify_commands import get_changed_files_against, verify + +ANSI = re.compile(r"\x1b\[[0-9;]*m") + + +@patch( Review Comment: `AGENTS.md` asks for `spec`/`autospec` when mocking — the `@patch` decorators here (and at lines 56, 69, 98 and 109) patch `get_changed_files_against` / `run_command` without `autospec=True`. ########## dev/breeze/doc/03_developer_tasks.rst: ########## @@ -316,6 +316,49 @@ If the provider name is ``apache-airflow-providers-cncf-kubernetes``, it will be Note: For building docs for apache-airflow-providers index, use ``apache-airflow-providers`` as the short hand operator. +Finding out what CI will run for your change Review Comment: The motivation is `AGENTS.md` step 7, but nothing points agents at `breeze verify` yet. Could you update step 7 in `AGENTS.md` (and the matching checklist in `.agents/skills/airflow-publish-changes/SKILL.md`) to use it? And a line in `dev/breeze/doc/ci/04_selective_checks.md`: adding a `run_*` flag to `SelectiveChecks` now also means classifying it in `verification_plan.py`, and that doc is where people look when they change selective checks. -- 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]
