jason810496 commented on code in PR #64209:
URL: https://github.com/apache/airflow/pull/64209#discussion_r3000690977
##########
airflow-core/tests/unit/core/test_configuration.py:
##########
@@ -1876,50 +1881,191 @@ 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
+
+ with conf.make_sure_configuration_loaded(with_providers=False):
+ assert conf._use_providers_configuration is False
+ with pytest.raises(
+ AirflowConfigException,
+ match=re.escape("section/key [celery/celery_app_name] not found in
config"),
+ ):
+ conf.get("celery", "celery_app_name")
+ # After the context manager exits, provider config is restored.
+ assert conf._use_providers_configuration is True
assert conf.get("celery", "celery_app_name") ==
"airflow.providers.celery.executors.celery_executor"
@skip_if_force_lowest_dependencies_marker
-def test_error_when_contributing_to_existing_section():
+def test_provider_sections_do_not_overlap_with_core():
+ """Test that provider config sections don't overlap with core
configuration sections."""
from airflow.settings import conf
- with conf.make_sure_configuration_loaded(with_providers=True):
- assert conf.providers_configuration_loaded is True
- 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
- conf.configuration_description["celery"] = {
- "description": "Celery Executor configuration",
- "options": {
- "celery_app_name": {
- "default": "test",
- }
- },
- }
- conf._default_values.add_section("celery")
- conf._default_values.set("celery", "celery_app_name", "test")
- assert conf.get("celery", "celery_app_name") == "test"
- # patching restoring_core_default_configuration to avoid reloading the
defaults
- with patch.object(conf, "restore_core_default_configuration"):
+ core_sections = set(conf._configuration_description.keys())
+ provider_sections =
set(conf._provider_metadata_configuration_description.keys())
+ overlap = core_sections & provider_sections
+ assert not overlap, (
+ f"Provider configuration sections overlap with core sections:
{overlap}. "
+ "Providers must only add new sections, not contribute to existing
ones."
+ )
+
+
+@skip_if_force_lowest_dependencies_marker
+class TestProviderConfigPriority:
+ """Tests that conf.get and conf.has_option respect provider metadata and
cfg fallbacks with correct priority.
+
+ Note: tests that use conf_vars must come AFTER tests that check
make_sure_configuration_loaded,
+ because conf_vars restores provider-sourced values into the config-file
layer, which then
+ persists even when providers are disabled.
Review Comment:
Addressed in 0058cc6.
I added helper function that return a new instance for test cases that might
mutate the internal state of `conf` to prevent the thread-safe issue.
--
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]
