potiuk commented on PR #72206:
URL: https://github.com/apache/airflow/pull/72206#issuecomment-5985218865
Thanks for the thorough review. All the suggestions are applied in
d0a258ba41: the shorter template comments (including keeping the
`metadata_db_environment` helper's comment to "only needed by components that
talk to the metadata DB"), a single `{{- if $keda }}` block, and the test
renames/docstrings. I also simplified
`test_metadata_db_env_absent_in_workers_by_default` to take the env names from
a single JMESPath search, and the init-container test now asserts directly on
the search result as you suggested. The branch is rebased on current main.
On the backport: I don't think we need one. Chart 2.0 drops support for
Airflow < 3.1.0 (#65866), so Airflow 2 deployments stay on the 1.x chart line,
which keeps shipping these variables — and this change shouldn't go there,
precisely because Airflow 2 workers still need them.
Could you take another look?
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk 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]
