shahar1 commented on code in PR #70724:
URL: https://github.com/apache/airflow/pull/70724#discussion_r4368409386
##########
.github/workflows/ci-amd-arm.yml:
##########
@@ -24,12 +24,18 @@ on: # yamllint disable-line rule:truthy
branches:
- v[0-9]+-[0-9]+-test
- providers-[a-z]+-?[a-z]*/v[0-9]+-[0-9]+
+ - chart/v[0-9]+-[0-9]+x-test
+ - airflow-ctl/v[0-9]+-[0-9]+-test
pull_request:
branches:
- main
- v[0-9]+-[0-9]+-test
- v[0-9]+-[0-9]+-stable
- providers-[a-z]+-?[a-z]*/v[0-9]+-[0-9]+
+ - chart/v[0-9]+-[0-9]+x-test
+ - chart/v[0-9]+-[0-9]+x-stable
+ - airflow-ctl/v[0-9]+-[0-9]+-test
+ - airflow-ctl/v[0-9]+-[0-9]+-stable
Review Comment:
**major** — the premise has moved since this was written. #75555 (merged
into `airflow-ctl/v0-1-test` today) added a dedicated `airflow-ctl-tests.yml`
that already runs on pull requests to `airflow-ctl/v*-test` and
`airflow-ctl/v*-stable`, plus the `CI passed` gate the release-branch ruleset
requires. Its `ci-passed` job needs only `tests-airflow-ctl`.
With this change on top, every PR to this branch would also start the full
`Tests (AMD/ARM)` workflow. On this PR's head that workflow is red (Static
checks, PROD image 3.10), and it is not part of `CI passed`, so it would
neither block nor gate auto-merge — it would just show a red X on every
backport.
Could you rebase on #75555 and decide which of the two is wanted here?
Either drop the `ci-amd-arm.yml` trigger changes and keep only CodeQL, or make
the full workflow green on this branch and add it to the gate (the
`check-ci-gate-needs` hook would then need to know about it).
##########
airflow-ctl-tests/tests/airflowctl_tests/test_airflowctl_commands.py:
##########
@@ -140,6 +134,19 @@ def date_param():
"plugins list-import-errors",
]
+# TODO: remove the skip once core exposes the asset state store API (the 3.2.x
core does not).
Review Comment:
**minor** — this branch is anchored on core 3.2.1 and will not gain the
asset state-store API, so the condition in this TODO (and in the
`asset_state_store.py` docstring, "Once core provides the API, move them back
into `generated.py`") can never be met here. The practical effect is that the
six commands backported in #72742 ship in 0.1.x with no integration coverage at
all.
The skip is a reasonable way to unblock CI, but it hides a release question
rather than answering it: should these commands be in a 0.1.x patch release
that only targets 3.2.x servers, or should #72742 be reverted from
`airflow-ctl/v0-1-test`? If they stay, I would reword the TODO to say they stay
skipped for the life of this branch, so nobody waits for a core change that
will not come.
--
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]
