Lee-W commented on code in PR #64209:
URL: https://github.com/apache/airflow/pull/64209#discussion_r2998765431
##########
airflow-core/tests/unit/core/test_configuration.py:
##########
@@ -1876,50 +1881,165 @@ def test_sensitive_values():
@skip_if_force_lowest_dependencies_marker
-def test_restore_and_reload_provider_configuration():
+def test_provider_configuration_toggle_with_context_manager():
+ """Test that make_sure_configuration_loaded toggles provider config
on/off."""
from airflow.settings import conf
- assert conf.providers_configuration_loaded is True
+ assert conf._use_providers_configuration is True
+ # With providers enabled, the provider value is returned via the fallback
lookup chain.
assert conf.get("celery", "celery_app_name") ==
"airflow.providers.celery.executors.celery_executor"
- conf.restore_core_default_configuration()
- assert conf.providers_configuration_loaded is False
- # built-in pre-2-7 celery executor
- assert conf.get("celery", "celery_app_name") ==
"airflow.executors.celery_executor"
- conf.load_providers_configuration()
- assert conf.providers_configuration_loaded is True
+ # When providers are excluded, the cfg fallback value (pre-2.7 path) is
used instead.
Review Comment:
It it's pre-2.7, do we still want to support it?
##########
scripts/in_container/run_check_default_configuration.py:
##########
@@ -36,23 +36,41 @@
if __name__ == "__main__":
with tempfile.TemporaryDirectory() as tmp_dir:
+ # We need to explicitly set the logging level to ERROR to avoid debug
logs from "airflow config lint" command that can spoil the output and make the
test fail.
+ # This is needed in case the default config has logging level set to
DEBUG, but it does not hurt to set it explicitly in any case to avoid any
unexpected debug logs from the command.
+ env = os.environ.copy()
+ env["AIRFLOW__LOGGING__LOGGING_LEVEL"] = "ERROR"
+
# Write default config cmd output to a temporary file
default_config_file = os.path.join(tmp_dir, "airflow.cfg")
with open(default_config_file, "w") as f:
- result = subprocess.run(list_default_config_cmd, check=False,
stdout=f)
+ result = subprocess.run(
+ list_default_config_cmd, check=False, stdout=f,
stderr=subprocess.PIPE, env=env
+ )
if result.returncode != 0:
print(f"\033[0;31mERROR: when running `{'
'.join(list_default_config_cmd)}`\033[0m\n")
+ if result.stderr:
+ print(result.stderr.decode())
+ print(f"Default config (if any) was written to:
{default_config_file}")
exit(1)
# Run airflow config lint to check the default config
- env = os.environ.copy()
env["AIRFLOW_HOME"] = tmp_dir
env["AIRFLOW_CONFIG"] = default_config_file
result = subprocess.run(lint_config_cmd, check=False,
capture_output=True, env=env)
- output: str = result.stdout.decode().strip()
- if result.returncode != 0 or expected_output not in output:
- print(f"\033[0;31mERROR: when running `{'
'.join(lint_config_cmd)}`\033[0m\n")
- print(output)
- exit(1)
- print(output)
- exit(0)
+ output: str = result.stdout.decode().strip()
+ if result.returncode != 0 or expected_output not in output:
+ print(f"\033[0;31mERROR: when running `{'
'.join(lint_config_cmd)}`\033[0m\n")
+ print(output)
+ # log the stderr as well if available
+ if result.stderr:
+ print(f"\033[0;31mERROR: stderr from `{'
'.join(lint_config_cmd)}`\033[0m\n")
+ print(result.stderr.decode())
+ # log the default config that was generated for debugging
+ print("\033[0;31mGenerated default config for debugging:\033[0m\n")
+ with open(default_config_file) as f:
+ print(f.read())
+ exit(1)
+ else:
+ print(output)
+ exit(0)
Review Comment:
```suggestion
print(output)
exit(0)
```
since the previous block will run `exit(1)`, we don't need this `else`
##########
shared/providers_discovery/src/airflow_shared/providers_discovery/providers_discovery.py:
##########
@@ -29,12 +30,11 @@
from time import perf_counter
from typing import Any, NamedTuple, ParamSpec, Protocol, cast
-import structlog
from packaging.utils import canonicalize_name
from ..module_loading import entry_points_with_dist
-log = structlog.getLogger(__name__)
+log = logging.getLogger(__name__)
Review Comment:
Why this change?
--
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]
