potiuk commented on code in PR #72209:
URL: https://github.com/apache/airflow/pull/72209#discussion_r4179641227
##########
chart/tests/helm_tests/airflow_aux/test_create_user_job.py:
##########
@@ -18,7 +18,34 @@
import jmespath
import pytest
-from chart_utils.helm_template_generator import render_chart
+from chart_utils.helm_template_generator import render_chart as _render_chart
+
+
+def _deep_merge(base: dict, override: dict) -> dict:
Review Comment:
Sorry, my mistake — the SHA in my reply was from before a rebase. The change
is `56ea433358` ("Make the create-user-job test opt-in explicit and trim the
docs") on the current branch: `render_chart_with_user_job` is the explicit
opt-in helper, the plain `render_chart` is used where the test is about the job
being off, and `test_should_not_create_job_by_default` renders the chart
untouched.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
##########
chart/values.yaml:
##########
@@ -1427,17 +1427,22 @@ scheduler:
# Airflow create user job settings
createUserJob:
- # Whether the create user job should be created
- enabled: true
+ # Whether the create user job should be created.
+ # Disabled by default: an account created here exists on every install with
the
+ # same credentials, so enable it only together with credentials of your own.
Review Comment:
Fair point — with the newsfragment and the major version bump, the history
doesn't need to live in values.yaml. Removed in 79d74ffdfe; the comment above
`enabled: false` is back to just what the option does.
---
Drafted-by: Claude Code (Opus 5); reviewed by @potiuk before posting
##########
chart/values.yaml:
##########
@@ -1433,17 +1433,19 @@ scheduler:
# Airflow create user job settings
createUserJob:
- # Whether the create user job should be created
- enabled: true
+ # Whether the create user job should be created.
+ # Creating a user here previously produced the same credentials on every
install.
+ enabled: false
- # Create initial user.
+ # Initial user to create. Only used when `createUserJob.enabled` is true,
and both
+ # `username` and `password` must be supplied
Review Comment:
Applied in 79d74ffdfe, thanks.
---
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]
