Miretpl commented on code in PR #72209:
URL: https://github.com/apache/airflow/pull/72209#discussion_r4495479657
##########
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:
> What I wanted to avoid is pasting the same four lines of opt-in into
thirty tests, which has its own maintenance cost
I understand and support that. What I don't agree with is how it was done.
We don't need the whole `_deep_merge` function here, which is pretty nice, but
is far too complicated IMHO for what we need here. Despite 2/3 tests, we can
create a static variable with config and add it directly in
`values={**static_value, ...}`, and for the remaining tests, do something
similar, just slightly differently.
--
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]
