Taragolis commented on code in PR #38188:
URL: https://github.com/apache/airflow/pull/38188#discussion_r1527149848
##########
tests/conftest.py:
##########
@@ -36,6 +37,34 @@
assert "airflow" not in sys.modules, "No airflow module can be imported before
these lines"
+# Clear all Environment Variables that might have side effect,
+# For example, defined in /files/airflow-breeze-config/variables.env
+_AIRFLOW_CONFIG_PATTERN = re.compile(r"^AIRFLOW__(.+)__(.+)$")
+_KEEP_CONFIGS_SETTINGS: dict[str, dict[str, set[str]]] = {
+ # Keep always these configurations
+ "always": {
+ "database": {"sql_alchemy_conn"},
+ "core": {"sql_alchemy_conn"},
Review Comment:
I guess we also need to remove pre-2.3 configuration option, but keep it for
a while, since it defined later in `conftest.py`
##########
tests/conftest.py:
##########
@@ -36,6 +37,34 @@
assert "airflow" not in sys.modules, "No airflow module can be imported before
these lines"
+# Clear all Environment Variables that might have side effect,
+# For example, defined in /files/airflow-breeze-config/variables.env
+_AIRFLOW_CONFIG_PATTERN = re.compile(r"^AIRFLOW__(.+)__(.+)$")
+_KEEP_CONFIGS_SETTINGS: dict[str, dict[str, set[str]]] = {
+ # Keep always these configurations
+ "always": {
+ "database": {"sql_alchemy_conn"},
+ "core": {"sql_alchemy_conn"},
+ "celery": {"result_backend", "broker_url"},
Review Comment:
I'm sure that we also need to check why unit tests expects to have this
parameters. If it required then I guess we need to move this tests into the
integration, if only for some test configuration, then better configure in
specific cases. But this just for thinking and maybe resolve as follow up
--
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: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
