Skip to content

Commit

Permalink
Emit warnings for conf.get* from the right source location (#28543)
Browse files Browse the repository at this point in the history 
`getboolean` and other typed get functions were issuing warnings from
"inside" themselves.

Before:

```
$ python ./airflow/airflow/kubernetes/kube_client.py
/home/ash/code/airflow/airflow/airflow/configuration.py:722 DeprecationWarning: The in_cluster option in [kubernetes] has been moved to the in_cluster option in [kubernetes_executor] - the old setting has been used, but please update your config.
```

After:

```
$ python ./airflow/airflow/kubernetes/kube_client.py
/home/ash/code/airflow/airflow/airflow/kubernetes/kube_client.py:89 DeprecationWarning: The in_cluster option in [kubernetes] has been moved to the in_cluster option in [kubernetes_executor] - the old setting has been used, but please update your config.
```

(cherry picked from commit f0ae250)
  • Loading branch information
@ashb @ephraimbuddy
ashb authored and ephraimbuddy committed Jan 12, 2023
1 parent 287dd0d commit 2a34ec4
Showing 1 changed file with 55 additions and 22 deletions.
77 changes: 55 additions & 22 deletions airflow/configuration.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ def _upgrade_postgres_metastore_conn(self):
must be replaced with `postgresql`.
"""
section, key = "database", "sql_alchemy_conn"
old_value = self.get(section, key)
old_value = self.get(section, key, _extra_stacklevel=1)
bad_schemes = ["postgres+psycopg2", "postgres"]
good_scheme = "postgresql"
parsed = urlsplit(old_value)
Expand Down Expand Up @@ -543,7 +543,7 @@ def _get_secret_option_from_config_sources(
return None

def get_mandatory_value(self, section: str, key: str, **kwargs) -> str:
value = self.get(section, key, **kwargs)
value = self.get(section, key, _extra_stacklevel=1, **kwargs)
if value is None:
raise ValueError(f"The value {section}/{key} should be set!")
return value
Expand All @@ -558,7 +558,13 @@ def get(self, section: str, key: str, **kwargs) -> str | None: # type: ignore[o

...

def get(self, section: str, key: str, **kwargs) -> str | None: # type: ignore[override, misc]
def get( # type: ignore[override, misc]
self,
section: str,
key: str,
_extra_stacklevel: int = 0,
**kwargs,
) -> str | None:
section = str(section).lower()
key = str(key).lower()
warning_emitted = False
Expand All @@ -574,7 +580,7 @@ def get(self, section: str, key: str, **kwargs) -> str | None: # type: ignore[o
f"The config section [{deprecated_section}] has been renamed to "
f"[{section}]. Please update your `conf.get*` call to use the new name",
FutureWarning,
stacklevel=2,
stacklevel=2 + _extra_stacklevel,
)
# Don't warn about individual rename if the whole section is renamed
warning_emitted = True
Expand All @@ -587,7 +593,7 @@ def get(self, section: str, key: str, **kwargs) -> str | None: # type: ignore[o
f"[{new_section}/{new_key}] instead. Please update your `conf.get*` call to use the "
"new name",
FutureWarning,
stacklevel=2,
stacklevel=2 + _extra_stacklevel,
)
warning_emitted = True
deprecated_section, deprecated_key = section, key
Expand All @@ -603,28 +609,49 @@ def get(self, section: str, key: str, **kwargs) -> str | None: # type: ignore[o

# first check environment variables
option = self._get_environment_variables(
deprecated_key, deprecated_section, key, section, issue_warning=not warning_emitted
deprecated_key,
deprecated_section,
key,
section,
issue_warning=not warning_emitted,
extra_stacklevel=_extra_stacklevel,
)
if option is not None:
return option

# ...then the config file
option = self._get_option_from_config_file(
deprecated_key, deprecated_section, key, kwargs, section, issue_warning=not warning_emitted
deprecated_key,
deprecated_section,
key,
kwargs,
section,
issue_warning=not warning_emitted,
extra_stacklevel=_extra_stacklevel,
)
if option is not None:
return option

# ...then commands
option = self._get_option_from_commands(
deprecated_key, deprecated_section, key, section, issue_warning=not warning_emitted
deprecated_key,
deprecated_section,
key,
section,
issue_warning=not warning_emitted,
extra_stacklevel=_extra_stacklevel,
)
if option is not None:
return option

# ...then from secret backends
option = self._get_option_from_secrets(
deprecated_key, deprecated_section, key, section, issue_warning=not warning_emitted
deprecated_key,
deprecated_section,
key,
section,
issue_warning=not warning_emitted,
extra_stacklevel=_extra_stacklevel,
)
if option is not None:
return option
Expand All @@ -644,6 +671,7 @@ def _get_option_from_secrets(
key: str,
section: str,
issue_warning: bool = True,
extra_stacklevel: int = 0,
) -> str | None:
option = self._get_secret_option(section, key)
if option:
Expand All @@ -653,7 +681,7 @@ def _get_option_from_secrets(
option = self._get_secret_option(deprecated_section, deprecated_key)
if option:
if issue_warning:
self._warn_deprecate(section, key, deprecated_section, deprecated_key)
self._warn_deprecate(section, key, deprecated_section, deprecated_key, extra_stacklevel)
return option
return None

Expand All @@ -664,6 +692,7 @@ def _get_option_from_commands(
key: str,
section: str,
issue_warning: bool = True,
extra_stacklevel: int = 0,
) -> str | None:
option = self._get_cmd_option(section, key)
if option:
Expand All @@ -673,7 +702,7 @@ def _get_option_from_commands(
option = self._get_cmd_option(deprecated_section, deprecated_key)
if option:
if issue_warning:
self._warn_deprecate(section, key, deprecated_section, deprecated_key)
self._warn_deprecate(section, key, deprecated_section, deprecated_key, extra_stacklevel)
return option
return None

Expand All @@ -685,6 +714,7 @@ def _get_option_from_config_file(
kwargs: dict[str, Any],
section: str,
issue_warning: bool = True,
extra_stacklevel: int = 0,
) -> str | None:
if super().has_option(section, key):
# Use the parent's methods to get the actual config here to be able to
Expand All @@ -693,7 +723,7 @@ def _get_option_from_config_file(
if deprecated_section and deprecated_key:
if super().has_option(deprecated_section, deprecated_key):
if issue_warning:
self._warn_deprecate(section, key, deprecated_section, deprecated_key)
self._warn_deprecate(section, key, deprecated_section, deprecated_key, extra_stacklevel)
with self.suppress_future_warnings():
return expand_env_var(super().get(deprecated_section, deprecated_key, **kwargs))
return None
Expand All @@ -705,6 +735,7 @@ def _get_environment_variables(
key: str,
section: str,
issue_warning: bool = True,
extra_stacklevel: int = 0,
) -> str | None:
option = self._get_env_var_option(section, key)
if option is not None:
Expand All @@ -714,12 +745,12 @@ def _get_environment_variables(
option = self._get_env_var_option(deprecated_section, deprecated_key)
if option is not None:
if issue_warning:
self._warn_deprecate(section, key, deprecated_section, deprecated_key)
self._warn_deprecate(section, key, deprecated_section, deprecated_key, extra_stacklevel)
return option
return None

def getboolean(self, section: str, key: str, **kwargs) -> bool: # type: ignore[override]
val = str(self.get(section, key, **kwargs)).lower().strip()
val = str(self.get(section, key, _extra_stacklevel=1, **kwargs)).lower().strip()
if "#" in val:
val = val.split("#")[0].strip()
if val in ("t", "true", "1"):
Expand All @@ -733,7 +764,7 @@ def getboolean(self, section: str, key: str, **kwargs) -> bool: # type: ignore[
)

def getint(self, section: str, key: str, **kwargs) -> int: # type: ignore[override]
val = self.get(section, key, **kwargs)
val = self.get(section, key, _extra_stacklevel=1, **kwargs)
if val is None:
raise AirflowConfigException(
f"Failed to convert value None to int. "
Expand All @@ -748,7 +779,7 @@ def getint(self, section: str, key: str, **kwargs) -> int: # type: ignore[overr
)

def getfloat(self, section: str, key: str, **kwargs) -> float: # type: ignore[override]
val = self.get(section, key, **kwargs)
val = self.get(section, key, _extra_stacklevel=1, **kwargs)
if val is None:
raise AirflowConfigException(
f"Failed to convert value None to float. "
Expand Down Expand Up @@ -799,7 +830,7 @@ def getjson(
fallback = _UNSET

try:
data = self.get(section=section, key=key, fallback=fallback, **kwargs)
data = self.get(section=section, key=key, fallback=fallback, _extra_stacklevel=1, **kwargs)
except (NoSectionError, NoOptionError):
return default

Expand All @@ -825,7 +856,7 @@ def gettimedelta(
:raises AirflowConfigException: raised because ValueError or OverflowError
:return: datetime.timedelta(seconds=<config_value>) or None
"""
val = self.get(section, key, fallback=fallback, **kwargs)
val = self.get(section, key, fallback=fallback, _extra_stacklevel=1, **kwargs)

if val:
# the given value must be convertible to integer
Expand Down Expand Up @@ -867,7 +898,7 @@ def has_option(self, section: str, option: str) -> bool:
# Using self.get() to avoid reimplementing the priority order
# of config variables (env, config, cmd, defaults)
# UNSET to avoid logging a warning about missing values
self.get(section, option, fallback=_UNSET)
self.get(section, option, fallback=_UNSET, _extra_stacklevel=1)
return True
except (NoOptionError, NoSectionError):
return False
Expand Down Expand Up @@ -1342,20 +1373,22 @@ def load_test_config(self):
self.read(TEST_CONFIG_FILE)

@staticmethod
def _warn_deprecate(section: str, key: str, deprecated_section: str, deprecated_name: str):
def _warn_deprecate(
section: str, key: str, deprecated_section: str, deprecated_name: str, extra_stacklevel: int
):
if section == deprecated_section:
warnings.warn(
f"The {deprecated_name} option in [{section}] has been renamed to {key} - "
f"the old setting has been used, but please update your config.",
DeprecationWarning,
stacklevel=4,
stacklevel=4 + extra_stacklevel,
)
else:
warnings.warn(
f"The {deprecated_name} option in [{deprecated_section}] has been moved to the {key} option "
f"in [{section}] - the old setting has been used, but please update your config.",
DeprecationWarning,
stacklevel=4,
stacklevel=4 + extra_stacklevel,
)

def __getstate__(self):
Expand Down

0 comments on commit 2a34ec4

Please sign in to comment.