You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/11/30 13:32:32 UTC
[GitHub] [airflow] blag commented on a diff in pull request #28008: Gracefully handle whole config sections being renamed
blag commented on code in PR #28008:
URL: https://github.com/apache/airflow/pull/28008#discussion_r1035973498
##########
airflow/configuration.py:
##########
@@ -572,63 +588,116 @@ def get(self, section: str, key: str, **kwargs) -> str | None: # type: ignore[o
section = str(section).lower()
key = str(key).lower()
- deprecated_section, deprecated_key, _ = self.deprecated_options.get(
- (section, key), (None, None, None)
- )
+ issue_warning = True
+ deprecated_section: str | None
+ deprecated_key: str | None
- option = self._get_environment_variables(deprecated_key, deprecated_section, key, section)
+ # For when we rename whole sections
+ if section in self.deprecated_sections:
+ warnings.warn(
+ f"The config section [{section}] has been renamed to {self.deprecated_sections[section][0]}."
+ " Please update your config.",
+ FutureWarning,
+ stacklevel=2,
+ )
+ # Don't warn about individual rename if the whole section is renamed
+ issue_warning = False
+ deprecated_section, deprecated_key = (section, key)
+ section = self.deprecated_sections[section][0]
+ elif (section, key) in self.inversed_deprecated_options:
+ # Handle using deprecated section/key instead of the new section/key
+ deprecated_section, deprecated_key = self.inversed_deprecated_options[(section, key)]
+ if issue_warning:
+ warnings.warn(
+ f"section/key [{deprecated_section}/{deprecated_key}] has been deprecated, "
+ f"you should use [{section}/{key}] instead.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
+ issue_warning = False
+ # deprecated_section, deprecated_key = section, key
+ # section, key = (new_section, new_key)
+ else:
+ deprecated_section, deprecated_key, _ = self.deprecated_options.get(
+ (section, key), (None, None, None)
+ )
+
+ # first check environment variables
+ option = self._get_environment_variables(
+ deprecated_key, deprecated_section, key, section, issue_warning=issue_warning
+ )
if option is not None:
return option
- option = self._get_option_from_config_file(deprecated_key, deprecated_section, key, kwargs, section)
+ # ...then the config file
+ option = self._get_option_from_config_file(
+ deprecated_key, deprecated_section, key, kwargs, section, issue_warning=issue_warning
+ )
if option is not None:
return option
- option = self._get_option_from_commands(deprecated_key, deprecated_section, key, section)
+ # ...then commands
+ option = self._get_option_from_commands(
+ deprecated_key, deprecated_section, key, section, issue_warning=issue_warning
+ )
if option is not None:
return option
- option = self._get_option_from_secrets(deprecated_key, deprecated_section, key, section)
+ # ...then from secret backends
+ option = self._get_option_from_secrets(
+ deprecated_key, deprecated_section, key, section, issue_warning=issue_warning
+ )
if option is not None:
return option
- return self._get_option_from_default_config(section, key, **kwargs)
-
- def _get_option_from_default_config(self, section: str, key: str, **kwargs) -> str | None:
# ...then the default config
if self.airflow_defaults.has_option(section, key) or "fallback" in kwargs:
return expand_env_var(self.airflow_defaults.get(section, key, **kwargs))
+ # Finally attempt: look for the old section name we have renamed en-mass
+ elif section in self.inversed_deprecated_sections:
+ old_section = self.inversed_deprecated_sections[section]
+ return self.get(old_section, key, **kwargs)
Review Comment:
This may need to be rewritten or removed, depending on the proposed changes above.
##########
airflow/configuration.py:
##########
@@ -572,63 +588,116 @@ def get(self, section: str, key: str, **kwargs) -> str | None: # type: ignore[o
section = str(section).lower()
key = str(key).lower()
- deprecated_section, deprecated_key, _ = self.deprecated_options.get(
- (section, key), (None, None, None)
- )
+ issue_warning = True
+ deprecated_section: str | None
+ deprecated_key: str | None
- option = self._get_environment_variables(deprecated_key, deprecated_section, key, section)
+ # For when we rename whole sections
+ if section in self.deprecated_sections:
Review Comment:
Hmmm, don't you want to check `inversed_deprecated_sections` here?
As it is, if somebody asks for `kubernetes.something`, it will check if `kubernetes` is in the keys of `self.deprecated_sections`, which it isn't (`kubernetes_executor` is).
##########
airflow/configuration.py:
##########
@@ -572,63 +588,116 @@ def get(self, section: str, key: str, **kwargs) -> str | None: # type: ignore[o
section = str(section).lower()
key = str(key).lower()
- deprecated_section, deprecated_key, _ = self.deprecated_options.get(
- (section, key), (None, None, None)
- )
+ issue_warning = True
+ deprecated_section: str | None
+ deprecated_key: str | None
- option = self._get_environment_variables(deprecated_key, deprecated_section, key, section)
+ # For when we rename whole sections
+ if section in self.deprecated_sections:
+ warnings.warn(
+ f"The config section [{section}] has been renamed to {self.deprecated_sections[section][0]}."
+ " Please update your config.",
+ FutureWarning,
+ stacklevel=2,
+ )
+ # Don't warn about individual rename if the whole section is renamed
+ issue_warning = False
+ deprecated_section, deprecated_key = (section, key)
+ section = self.deprecated_sections[section][0]
+ elif (section, key) in self.inversed_deprecated_options:
+ # Handle using deprecated section/key instead of the new section/key
+ deprecated_section, deprecated_key = self.inversed_deprecated_options[(section, key)]
+ if issue_warning:
+ warnings.warn(
+ f"section/key [{deprecated_section}/{deprecated_key}] has been deprecated, "
+ f"you should use [{section}/{key}] instead.",
+ DeprecationWarning,
+ stacklevel=2,
+ )
+ issue_warning = False
+ # deprecated_section, deprecated_key = section, key
+ # section, key = (new_section, new_key)
Review Comment:
```suggestion
```
--
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-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org