potiuk commented on PR #72209: URL: https://github.com/apache/airflow/pull/72209#issuecomment-5730863555
Rebased onto current `main`; full CI is green (142 checks, nothing failing). All six review points are addressed, four of them exactly as suggested — the production-guide wording and the security note were taken verbatim, and both `values.yaml` comments were cut back. Two I partly pushed back on, and those replies are still open for you to agree or not: - **`NOTES.txt`** — shortened to one line and a pointer rather than removed, because it only renders when `createUserJob.enabled` is false, which after this change is every default install; deleting it leaves a fresh install with no UI login and no hint why. - **`values.yaml`** — dropped the "disabled by default" restatement, kept one line recording that the old defaults produced identical credentials on every install, since that is the reason the default is flipping. The test wrapper is gone rather than argued for: `render_chart` keeps its own name, the helper is now `render_chart_with_user_job`, and `test_should_run_by_default` — which asserted the opposite of the new behaviour — is replaced by `test_should_run_when_enabled_with_credentials` plus a new `test_should_not_create_job_by_default`. --- 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]
