Copilot commented on code in PR #62696:
URL: https://github.com/apache/airflow/pull/62696#discussion_r2973028311
##########
task-sdk/src/airflow/sdk/configuration.py:
##########
@@ -32,6 +32,7 @@
configure_parser_from_configuration_description,
)
from airflow.sdk.execution_time.secrets import
_SERVER_DEFAULT_SECRETS_SEARCH_PATH
+from airflow.sdk.providers_manager_runtime import ProvidersManagerTaskRuntime
Review Comment:
Importing `ProvidersManagerTaskRuntime` at module import time defeats the
lazy `conf` initialization pattern used in this module (`conf` is created via
`__getattr__`). This will pull in `providers_manager_runtime` (and its
dependencies) even for callers that import `airflow.sdk.configuration` but
never access `conf`. Consider delaying this import until
`AirflowSDKConfigParser.__init__`/`initialize_config()` (or passing a
lightweight factory) so importing the module stays cheap when `conf` is unused.
##########
shared/configuration/src/airflow_shared/configuration/parser.py:
##########
@@ -241,8 +275,75 @@ def _lookup_sequence(self) -> list[Callable]:
self._get_option_from_commands,
self._get_option_from_secrets,
self._get_option_from_defaults,
+ self._get_option_from_provider_cfg_config_fallbacks,
+ self._get_option_from_provider_metadata_config_fallbacks,
]
+ def _get_config_sources_for_as_dict(self) -> list[tuple[str,
ConfigParser]]:
+ """Override the base method to add provider fallbacks."""
+ return [
+ ("provider-cfg-fallback-defaults",
self._provider_cfg_config_fallback_default_values),
+ ("provider-metadata-fallback-defaults",
self._provider_metadata_config_fallback_default_values),
+ ("default", self._default_values),
+ ("airflow.cfg", self),
+ ]
Review Comment:
`_get_config_sources_for_as_dict()` now unconditionally includes provider
fallback sources. This can cause provider discovery/config merging side effects
when `as_dict()` is called (notably `write()` calls
`as_dict(display_source=True)` before entering
`make_sure_configuration_loaded(with_providers=include_providers)`). As a
result, `write(..., include_providers=False, include_sources=True)` may still
initialize providers and/or include provider-derived sections/sources
unexpectedly. Consider gating these provider sources on whether providers are
actually requested/loaded (e.g., only when providers configuration is loaded,
or by computing `sources_dict` inside the `make_sure_configuration_loaded`
context).
```suggestion
"""Override the base method to add provider fallbacks when providers
are loaded."""
# Always include core configuration sources.
sources: list[tuple[str, ConfigParser]] = [
("default", self._default_values),
("airflow.cfg", self),
]
# Optionally include provider fallback sources only if provider
configuration is loaded.
if getattr(self, "_providers_configuration_loaded", False):
sources.insert(0, ("provider-metadata-fallback-defaults",
self._provider_metadata_config_fallback_default_values))
sources.insert(0, ("provider-cfg-fallback-defaults",
self._provider_cfg_config_fallback_default_values))
return sources
```
--
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]
