You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by je...@apache.org on 2022/04/22 17:46:14 UTC
[airflow] branch main updated: Fix deprecated and updated env var config handling (#23137)
This is an automated email from the ASF dual-hosted git repository.
jedcunningham pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 89ecb64893 Fix deprecated and updated env var config handling (#23137)
89ecb64893 is described below
commit 89ecb64893a6f79bf1da85d10e3a43dae0efd43d
Author: Jed Cunningham <66...@users.noreply.github.com>
AuthorDate: Fri Apr 22 11:46:00 2022 -0600
Fix deprecated and updated env var config handling (#23137)
Configs that both are have a deprecated ancestor and need to modified,
for example `auth_backend` -> `auth_backends` + `session` backend, were
broken when the old key was used via an env var.
`get` looks for the deprecated key also and finds it, leading to the
unmodified value being used instead of the modified one. Removing the
old env var to avoid this issue.
We also start setting the new value in an env var so subprocesses see it.
This surfaced a bug with `log_filename_template`. We now set the
`parsed` value in the modified value, as it ends up in an env var so the
modification works properly in subprocesses also.
---
airflow/configuration.py | 26 +++++++++++++++-----------
tests/core/test_configuration.py | 38 ++++++++++++++++++++++++++++++++++++++
2 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/airflow/configuration.py b/airflow/configuration.py
index d31933779d..3c696bd090 100644
--- a/airflow/configuration.py
+++ b/airflow/configuration.py
@@ -271,7 +271,7 @@ class AirflowConfigParser(ConfigParser):
if default_config is not None:
self.airflow_defaults.read_string(default_config)
# Set the upgrade value based on the current loaded default
- default = self.airflow_defaults.get('logging', 'log_filename_template', fallback=None, raw=True)
+ default = self.airflow_defaults.get('logging', 'log_filename_template', fallback=None)
if default:
replacement = self.deprecated_values['logging']['log_filename_template']
self.deprecated_values['logging']['log_filename_template'] = (
@@ -290,9 +290,7 @@ class AirflowConfigParser(ConfigParser):
self.is_validated = False
def validate(self):
-
self._validate_config_dependencies()
-
self._validate_enums()
for section, replacement in self.deprecated_values.items():
@@ -327,6 +325,13 @@ class AirflowConfigParser(ConfigParser):
elif old_value.find('airflow.api.auth.backend.session') == -1:
new_value = old_value + ",airflow.api.auth.backend.session"
self._update_env_var(section="api", name="auth_backends", new_value=new_value)
+ self.upgraded_values[("api", "auth_backends")] = old_value
+
+ # if the old value is set via env var, we need to wipe it
+ # otherwise, it'll "win" over our adjusted value
+ old_env_var = self._env_var_name("api", "auth_backend")
+ os.environ.pop(old_env_var, None)
+
warnings.warn(
'The auth_backends setting in [api] has had airflow.api.auth.backend.session added '
'in the running config, which is needed by the UI. Please update your config before '
@@ -351,7 +356,11 @@ class AirflowConfigParser(ConfigParser):
self.upgraded_values[(section, key)] = old_value
new_value = re.sub('^' + re.escape(f"{bad_scheme}://"), f"{good_scheme}://", old_value)
self._update_env_var(section=section, name=key, new_value=new_value)
- self.set(section=section, option=key, value=new_value)
+
+ # if the old value is set via env var, we need to wipe it
+ # otherwise, it'll "win" over our adjusted value
+ old_env_var = self._env_var_name("core", key)
+ os.environ.pop(old_env_var, None)
def _validate_enums(self):
"""Validate that enum type config has an accepted value"""
@@ -395,13 +404,8 @@ class AirflowConfigParser(ConfigParser):
def _update_env_var(self, section, name, new_value):
env_var = self._env_var_name(section, name)
- # If the config comes from environment, set it there so that any subprocesses keep the same override!
- if env_var in os.environ:
- os.environ[env_var] = new_value
- return
- if not self.has_section(section):
- self.add_section(section)
- self.set(section, name, new_value)
+ # Set it as an env var so that any subprocesses keep the same override!
+ os.environ[env_var] = new_value
@staticmethod
def _create_future_warning(name, section, current_value, new_value, version):
diff --git a/tests/core/test_configuration.py b/tests/core/test_configuration.py
index e6c0a1546e..70a51806c4 100644
--- a/tests/core/test_configuration.py
+++ b/tests/core/test_configuration.py
@@ -616,6 +616,44 @@ AIRFLOW_HOME = /root/airflow
== 'airflow.api.auth.backend.basic_auth,airflow.api.auth.backend.session'
)
+ @pytest.mark.parametrize(
+ "old, new",
+ [
+ (
+ ("api", "auth_backend", "airflow.api.auth.backend.basic_auth"),
+ (
+ "api",
+ "auth_backends",
+ "airflow.api.auth.backend.basic_auth,airflow.api.auth.backend.session",
+ ),
+ ),
+ (
+ ("core", "sql_alchemy_conn", "postgres+psycopg2://localhost/postgres"),
+ ("database", "sql_alchemy_conn", "postgresql://localhost/postgres"),
+ ),
+ ],
+ )
+ def test_deprecated_env_vars_upgraded_and_removed(self, old, new):
+ test_conf = AirflowConfigParser(default_config='')
+ old_section, old_key, old_value = old
+ new_section, new_key, new_value = new
+ old_env_var = test_conf._env_var_name(old_section, old_key)
+ new_env_var = test_conf._env_var_name(new_section, new_key)
+
+ with pytest.warns(FutureWarning):
+ with unittest.mock.patch.dict('os.environ', **{old_env_var: old_value}):
+ # Can't start with the new env var existing...
+ os.environ.pop(new_env_var, None)
+
+ test_conf.validate()
+ assert test_conf.get(new_section, new_key) == new_value
+ # We also need to make sure the deprecated env var is removed
+ # so that any subprocesses don't use it in place of our updated
+ # value.
+ assert old_env_var not in os.environ
+ # and make sure we track the old value as well, under the new section/key
+ assert test_conf.upgraded_values[(new_section, new_key)] == old_value
+
@pytest.mark.parametrize(
"conf_dict",
[