From fcfba5f12efb58b5182da448b29c085bc594e60b Mon Sep 17 00:00:00 2001 From: viiccwen Date: Sat, 16 Aug 2025 01:12:18 +0800 Subject: [PATCH 01/11] fix(logging): fix module loading by using `import_module()` - Replace `import_string()` with `import_module()` in `load_logging_config()` - Fix issue where simple module paths like `log_config.LOGGING_CONFIG` failed - Add `importlib` import inside function to avoid circular imports --- airflow-core/src/airflow/logging_config.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/airflow-core/src/airflow/logging_config.py b/airflow-core/src/airflow/logging_config.py index e6d837bc22077..82e94a8ed2944 100644 --- a/airflow-core/src/airflow/logging_config.py +++ b/airflow-core/src/airflow/logging_config.py @@ -73,7 +73,10 @@ def load_logging_config() -> tuple[dict[str, Any], str]: else: modpath = logging_class_path.rsplit(".", 1)[0] try: - mod = import_string(modpath) + # Import here to avoid circular imports + from importlib import import_module + + mod = import_module(modpath) REMOTE_TASK_LOG = getattr(mod, "REMOTE_TASK_LOG") DEFAULT_REMOTE_CONN_ID = getattr(mod, "DEFAULT_REMOTE_CONN_ID", None) except Exception as err: From fa2fccecdabe786e5caefd2e4dbe329b55f26611 Mon Sep 17 00:00:00 2001 From: viiccwen Date: Sat, 16 Aug 2025 01:13:48 +0800 Subject: [PATCH 02/11] test(logging): add tests for logging config module path handling - Add parametrized test for simple and nested module paths - Add test for graceful fallback when remote logging vars are missing - Create helper method to reduce test code duplication - Test both log_config.LOGGING_CONFIG and nested.module.LOGGING_CONFIG scenarios Ensures logging config works with various module path structures --- .../tests/unit/core/test_logging_config.py | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/airflow-core/tests/unit/core/test_logging_config.py b/airflow-core/tests/unit/core/test_logging_config.py index 72864b066b34a..60db2a80760a2 100644 --- a/airflow-core/tests/unit/core/test_logging_config.py +++ b/airflow-core/tests/unit/core/test_logging_config.py @@ -96,6 +96,108 @@ # Other settings here """ +SETTINGS_FILE_SIMPLE_MODULE = """ +LOGGING_CONFIG = { + 'version': 1, + 'disable_existing_loggers': False, + 'formatters': { + 'airflow.task': { + 'format': '[%(asctime)s] {%(filename)s:%(lineno)d} %(levelname)s - %(message)s' + }, + }, + 'handlers': { + 'console': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + 'task': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + }, + 'loggers': { + 'airflow.task': { + 'handlers': ['task'], + 'level': 'INFO', + 'propagate': False, + }, + } +} + +# Test remote logging variables +REMOTE_TASK_LOG = None +DEFAULT_REMOTE_CONN_ID = "test_conn_id" +""" + +SETTINGS_FILE_NESTED_MODULE = """ +LOGGING_CONFIG = { + 'version': 1, + 'disable_existing_loggers': False, + 'formatters': { + 'airflow.task': { + 'format': '[%(asctime)s] {%(filename)s:%(lineno)d} %(levelname)s - %(message)s' + }, + }, + 'handlers': { + 'console': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + 'task': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + }, + 'loggers': { + 'airflow.task': { + 'handlers': ['task'], + 'level': 'INFO', + 'propagate': False, + }, + } +} + +# Test remote logging variables +REMOTE_TASK_LOG = None +DEFAULT_REMOTE_CONN_ID = "nested_conn_id" +""" + +SETTINGS_FILE_NO_REMOTE_VARS = """ +LOGGING_CONFIG = { + 'version': 1, + 'disable_existing_loggers': False, + 'formatters': { + 'airflow.task': { + 'format': '[%(asctime)s] {%(filename)s:%(lineno)d} %(levelname)s - %(message)s' + }, + }, + 'handlers': { + 'console': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + 'task': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + }, + 'loggers': { + 'airflow.task': { + 'handlers': ['task'], + 'level': 'INFO', + 'propagate': False, + }, + } +} +# Note: No REMOTE_TASK_LOG or DEFAULT_REMOTE_CONN_ID defined +""" + SETTINGS_DEFAULT_NAME = "custom_airflow_local_settings" @@ -191,6 +293,13 @@ def teardown_method(self): importlib.reload(airflow_local_settings) configure_logging() + def _verify_basic_logging_config(self, logging_config, logging_class_path, expected_path): + """Helper method to verify basic logging config structure""" + assert isinstance(logging_config, dict) + assert logging_config["version"] == 1 + assert "airflow.task" in logging_config["loggers"] + assert logging_class_path == expected_path + # When we try to load an invalid config file, we expect an error def test_loading_invalid_local_settings(self): from airflow.logging_config import configure_logging, log @@ -365,3 +474,40 @@ def test_loading_remote_logging_with_hdfs_handler(self): airflow.logging_config.configure_logging() assert isinstance(airflow.logging_config.REMOTE_TASK_LOG, HdfsRemoteLogIO) + + @pytest.mark.parametrize( + "settings_content,module_structure,expected_path", + [ + (SETTINGS_FILE_SIMPLE_MODULE, None, f"{SETTINGS_DEFAULT_NAME}.LOGGING_CONFIG"), + ( + SETTINGS_FILE_NESTED_MODULE, + "nested.config.module", + f"nested.config.module.{SETTINGS_DEFAULT_NAME}.LOGGING_CONFIG", + ), + ], + ) + def test_load_logging_config_module_paths(self, settings_content, module_structure, expected_path): + """Test that load_logging_config works with different module path structures""" + dir_structure = module_structure.replace(".", "/") if module_structure else None + + with settings_context(settings_content, dir_structure): + from airflow.logging_config import load_logging_config + + logging_config, logging_class_path = load_logging_config() + self._verify_basic_logging_config(logging_config, logging_class_path, expected_path) + + def test_load_logging_config_fallback_behavior(self): + """Test that load_logging_config falls back gracefully when remote logging vars are missing""" + with settings_context(SETTINGS_FILE_NO_REMOTE_VARS): + from airflow.logging_config import load_logging_config + + with patch("airflow.logging_config.log") as mock_log: + logging_config, _ = load_logging_config() + + assert isinstance(logging_config, dict) + assert logging_config["version"] == 1 + + mock_log.info.assert_called_with( + "Remote task logs will not be available due to an error: %s", + mock_log.info.call_args[0][1], + ) From 79b755395e1f55493f7a4825a8fe448e6d09e6b2 Mon Sep 17 00:00:00 2001 From: viiccwen Date: Sat, 16 Aug 2025 01:12:18 +0800 Subject: [PATCH 03/11] fix(logging): fix module loading by using `import_module()` - Replace `import_string()` with `import_module()` in `load_logging_config()` - Fix issue where simple module paths like `log_config.LOGGING_CONFIG` failed - Add `importlib` import inside function to avoid circular imports --- airflow-core/src/airflow/logging_config.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/airflow-core/src/airflow/logging_config.py b/airflow-core/src/airflow/logging_config.py index e6d837bc22077..82e94a8ed2944 100644 --- a/airflow-core/src/airflow/logging_config.py +++ b/airflow-core/src/airflow/logging_config.py @@ -73,7 +73,10 @@ def load_logging_config() -> tuple[dict[str, Any], str]: else: modpath = logging_class_path.rsplit(".", 1)[0] try: - mod = import_string(modpath) + # Import here to avoid circular imports + from importlib import import_module + + mod = import_module(modpath) REMOTE_TASK_LOG = getattr(mod, "REMOTE_TASK_LOG") DEFAULT_REMOTE_CONN_ID = getattr(mod, "DEFAULT_REMOTE_CONN_ID", None) except Exception as err: From f90682cd77557151ef515c56e36712b6798c674e Mon Sep 17 00:00:00 2001 From: viiccwen Date: Sat, 16 Aug 2025 01:13:48 +0800 Subject: [PATCH 04/11] test(logging): add tests for logging config module path handling - Add parametrized test for simple and nested module paths - Add test for graceful fallback when remote logging vars are missing - Create helper method to reduce test code duplication - Test both log_config.LOGGING_CONFIG and nested.module.LOGGING_CONFIG scenarios Ensures logging config works with various module path structures --- .../tests/unit/core/test_logging_config.py | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/airflow-core/tests/unit/core/test_logging_config.py b/airflow-core/tests/unit/core/test_logging_config.py index 72864b066b34a..60db2a80760a2 100644 --- a/airflow-core/tests/unit/core/test_logging_config.py +++ b/airflow-core/tests/unit/core/test_logging_config.py @@ -96,6 +96,108 @@ # Other settings here """ +SETTINGS_FILE_SIMPLE_MODULE = """ +LOGGING_CONFIG = { + 'version': 1, + 'disable_existing_loggers': False, + 'formatters': { + 'airflow.task': { + 'format': '[%(asctime)s] {%(filename)s:%(lineno)d} %(levelname)s - %(message)s' + }, + }, + 'handlers': { + 'console': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + 'task': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + }, + 'loggers': { + 'airflow.task': { + 'handlers': ['task'], + 'level': 'INFO', + 'propagate': False, + }, + } +} + +# Test remote logging variables +REMOTE_TASK_LOG = None +DEFAULT_REMOTE_CONN_ID = "test_conn_id" +""" + +SETTINGS_FILE_NESTED_MODULE = """ +LOGGING_CONFIG = { + 'version': 1, + 'disable_existing_loggers': False, + 'formatters': { + 'airflow.task': { + 'format': '[%(asctime)s] {%(filename)s:%(lineno)d} %(levelname)s - %(message)s' + }, + }, + 'handlers': { + 'console': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + 'task': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + }, + 'loggers': { + 'airflow.task': { + 'handlers': ['task'], + 'level': 'INFO', + 'propagate': False, + }, + } +} + +# Test remote logging variables +REMOTE_TASK_LOG = None +DEFAULT_REMOTE_CONN_ID = "nested_conn_id" +""" + +SETTINGS_FILE_NO_REMOTE_VARS = """ +LOGGING_CONFIG = { + 'version': 1, + 'disable_existing_loggers': False, + 'formatters': { + 'airflow.task': { + 'format': '[%(asctime)s] {%(filename)s:%(lineno)d} %(levelname)s - %(message)s' + }, + }, + 'handlers': { + 'console': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + 'task': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + }, + 'loggers': { + 'airflow.task': { + 'handlers': ['task'], + 'level': 'INFO', + 'propagate': False, + }, + } +} +# Note: No REMOTE_TASK_LOG or DEFAULT_REMOTE_CONN_ID defined +""" + SETTINGS_DEFAULT_NAME = "custom_airflow_local_settings" @@ -191,6 +293,13 @@ def teardown_method(self): importlib.reload(airflow_local_settings) configure_logging() + def _verify_basic_logging_config(self, logging_config, logging_class_path, expected_path): + """Helper method to verify basic logging config structure""" + assert isinstance(logging_config, dict) + assert logging_config["version"] == 1 + assert "airflow.task" in logging_config["loggers"] + assert logging_class_path == expected_path + # When we try to load an invalid config file, we expect an error def test_loading_invalid_local_settings(self): from airflow.logging_config import configure_logging, log @@ -365,3 +474,40 @@ def test_loading_remote_logging_with_hdfs_handler(self): airflow.logging_config.configure_logging() assert isinstance(airflow.logging_config.REMOTE_TASK_LOG, HdfsRemoteLogIO) + + @pytest.mark.parametrize( + "settings_content,module_structure,expected_path", + [ + (SETTINGS_FILE_SIMPLE_MODULE, None, f"{SETTINGS_DEFAULT_NAME}.LOGGING_CONFIG"), + ( + SETTINGS_FILE_NESTED_MODULE, + "nested.config.module", + f"nested.config.module.{SETTINGS_DEFAULT_NAME}.LOGGING_CONFIG", + ), + ], + ) + def test_load_logging_config_module_paths(self, settings_content, module_structure, expected_path): + """Test that load_logging_config works with different module path structures""" + dir_structure = module_structure.replace(".", "/") if module_structure else None + + with settings_context(settings_content, dir_structure): + from airflow.logging_config import load_logging_config + + logging_config, logging_class_path = load_logging_config() + self._verify_basic_logging_config(logging_config, logging_class_path, expected_path) + + def test_load_logging_config_fallback_behavior(self): + """Test that load_logging_config falls back gracefully when remote logging vars are missing""" + with settings_context(SETTINGS_FILE_NO_REMOTE_VARS): + from airflow.logging_config import load_logging_config + + with patch("airflow.logging_config.log") as mock_log: + logging_config, _ = load_logging_config() + + assert isinstance(logging_config, dict) + assert logging_config["version"] == 1 + + mock_log.info.assert_called_with( + "Remote task logs will not be available due to an error: %s", + mock_log.info.call_args[0][1], + ) From 4faf0320ba2e2af382488ca3f10d6a9d47eca956 Mon Sep 17 00:00:00 2001 From: viiccwen Date: Sat, 16 Aug 2025 01:12:18 +0800 Subject: [PATCH 05/11] fix(logging): fix module loading by using `import_module()` - Replace `import_string()` with `import_module()` in `load_logging_config()` - Fix issue where simple module paths like `log_config.LOGGING_CONFIG` failed - Add `importlib` import inside function to avoid circular imports --- airflow-core/src/airflow/logging_config.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/airflow-core/src/airflow/logging_config.py b/airflow-core/src/airflow/logging_config.py index e6d837bc22077..82e94a8ed2944 100644 --- a/airflow-core/src/airflow/logging_config.py +++ b/airflow-core/src/airflow/logging_config.py @@ -73,7 +73,10 @@ def load_logging_config() -> tuple[dict[str, Any], str]: else: modpath = logging_class_path.rsplit(".", 1)[0] try: - mod = import_string(modpath) + # Import here to avoid circular imports + from importlib import import_module + + mod = import_module(modpath) REMOTE_TASK_LOG = getattr(mod, "REMOTE_TASK_LOG") DEFAULT_REMOTE_CONN_ID = getattr(mod, "DEFAULT_REMOTE_CONN_ID", None) except Exception as err: From 347f8b1534011b0d687acbd1c90982e0b6d69515 Mon Sep 17 00:00:00 2001 From: viiccwen Date: Sat, 16 Aug 2025 01:13:48 +0800 Subject: [PATCH 06/11] test(logging): add tests for logging config module path handling - Add parametrized test for simple and nested module paths - Add test for graceful fallback when remote logging vars are missing - Create helper method to reduce test code duplication - Test both log_config.LOGGING_CONFIG and nested.module.LOGGING_CONFIG scenarios Ensures logging config works with various module path structures --- .../tests/unit/core/test_logging_config.py | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) diff --git a/airflow-core/tests/unit/core/test_logging_config.py b/airflow-core/tests/unit/core/test_logging_config.py index 72864b066b34a..60db2a80760a2 100644 --- a/airflow-core/tests/unit/core/test_logging_config.py +++ b/airflow-core/tests/unit/core/test_logging_config.py @@ -96,6 +96,108 @@ # Other settings here """ +SETTINGS_FILE_SIMPLE_MODULE = """ +LOGGING_CONFIG = { + 'version': 1, + 'disable_existing_loggers': False, + 'formatters': { + 'airflow.task': { + 'format': '[%(asctime)s] {%(filename)s:%(lineno)d} %(levelname)s - %(message)s' + }, + }, + 'handlers': { + 'console': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + 'task': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + }, + 'loggers': { + 'airflow.task': { + 'handlers': ['task'], + 'level': 'INFO', + 'propagate': False, + }, + } +} + +# Test remote logging variables +REMOTE_TASK_LOG = None +DEFAULT_REMOTE_CONN_ID = "test_conn_id" +""" + +SETTINGS_FILE_NESTED_MODULE = """ +LOGGING_CONFIG = { + 'version': 1, + 'disable_existing_loggers': False, + 'formatters': { + 'airflow.task': { + 'format': '[%(asctime)s] {%(filename)s:%(lineno)d} %(levelname)s - %(message)s' + }, + }, + 'handlers': { + 'console': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + 'task': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + }, + 'loggers': { + 'airflow.task': { + 'handlers': ['task'], + 'level': 'INFO', + 'propagate': False, + }, + } +} + +# Test remote logging variables +REMOTE_TASK_LOG = None +DEFAULT_REMOTE_CONN_ID = "nested_conn_id" +""" + +SETTINGS_FILE_NO_REMOTE_VARS = """ +LOGGING_CONFIG = { + 'version': 1, + 'disable_existing_loggers': False, + 'formatters': { + 'airflow.task': { + 'format': '[%(asctime)s] {%(filename)s:%(lineno)d} %(levelname)s - %(message)s' + }, + }, + 'handlers': { + 'console': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + 'task': { + 'class': 'logging.StreamHandler', + 'formatter': 'airflow.task', + 'stream': 'ext://sys.stdout' + }, + }, + 'loggers': { + 'airflow.task': { + 'handlers': ['task'], + 'level': 'INFO', + 'propagate': False, + }, + } +} +# Note: No REMOTE_TASK_LOG or DEFAULT_REMOTE_CONN_ID defined +""" + SETTINGS_DEFAULT_NAME = "custom_airflow_local_settings" @@ -191,6 +293,13 @@ def teardown_method(self): importlib.reload(airflow_local_settings) configure_logging() + def _verify_basic_logging_config(self, logging_config, logging_class_path, expected_path): + """Helper method to verify basic logging config structure""" + assert isinstance(logging_config, dict) + assert logging_config["version"] == 1 + assert "airflow.task" in logging_config["loggers"] + assert logging_class_path == expected_path + # When we try to load an invalid config file, we expect an error def test_loading_invalid_local_settings(self): from airflow.logging_config import configure_logging, log @@ -365,3 +474,40 @@ def test_loading_remote_logging_with_hdfs_handler(self): airflow.logging_config.configure_logging() assert isinstance(airflow.logging_config.REMOTE_TASK_LOG, HdfsRemoteLogIO) + + @pytest.mark.parametrize( + "settings_content,module_structure,expected_path", + [ + (SETTINGS_FILE_SIMPLE_MODULE, None, f"{SETTINGS_DEFAULT_NAME}.LOGGING_CONFIG"), + ( + SETTINGS_FILE_NESTED_MODULE, + "nested.config.module", + f"nested.config.module.{SETTINGS_DEFAULT_NAME}.LOGGING_CONFIG", + ), + ], + ) + def test_load_logging_config_module_paths(self, settings_content, module_structure, expected_path): + """Test that load_logging_config works with different module path structures""" + dir_structure = module_structure.replace(".", "/") if module_structure else None + + with settings_context(settings_content, dir_structure): + from airflow.logging_config import load_logging_config + + logging_config, logging_class_path = load_logging_config() + self._verify_basic_logging_config(logging_config, logging_class_path, expected_path) + + def test_load_logging_config_fallback_behavior(self): + """Test that load_logging_config falls back gracefully when remote logging vars are missing""" + with settings_context(SETTINGS_FILE_NO_REMOTE_VARS): + from airflow.logging_config import load_logging_config + + with patch("airflow.logging_config.log") as mock_log: + logging_config, _ = load_logging_config() + + assert isinstance(logging_config, dict) + assert logging_config["version"] == 1 + + mock_log.info.assert_called_with( + "Remote task logs will not be available due to an error: %s", + mock_log.info.call_args[0][1], + ) From f7c2972046d3e63ef8c6e448d2dfef61f5349493 Mon Sep 17 00:00:00 2001 From: viiccwen Date: Sat, 16 Aug 2025 17:07:11 +0800 Subject: [PATCH 07/11] refactor(logging): move `import_module` to the top level --- airflow-core/src/airflow/logging_config.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/airflow-core/src/airflow/logging_config.py b/airflow-core/src/airflow/logging_config.py index 82e94a8ed2944..8ae7b50bc82c6 100644 --- a/airflow-core/src/airflow/logging_config.py +++ b/airflow-core/src/airflow/logging_config.py @@ -24,7 +24,7 @@ from airflow.configuration import conf from airflow.exceptions import AirflowConfigException -from airflow.utils.module_loading import import_string +from airflow.utils.module_loading import import_module, import_string if TYPE_CHECKING: from airflow.logging.remote import RemoteLogIO @@ -73,10 +73,9 @@ def load_logging_config() -> tuple[dict[str, Any], str]: else: modpath = logging_class_path.rsplit(".", 1)[0] try: - # Import here to avoid circular imports - from importlib import import_module - mod = import_module(modpath) + + # Load remote logging configuration from the custom module REMOTE_TASK_LOG = getattr(mod, "REMOTE_TASK_LOG") DEFAULT_REMOTE_CONN_ID = getattr(mod, "DEFAULT_REMOTE_CONN_ID", None) except Exception as err: From 18c04a2797bb6d79e09d7396ed22dff56f7e3493 Mon Sep 17 00:00:00 2001 From: viiccwen Date: Sat, 16 Aug 2025 17:11:05 +0800 Subject: [PATCH 08/11] test(logging): optimize test constants to reduce code duplication Replace `SETTINGS_FILE_NESTED_MODULE` replicate part with string replacement from `SETTINGS_FILE_SIMPLE_MODULE` --- .../tests/unit/core/test_logging_config.py | 43 +++---------------- 1 file changed, 7 insertions(+), 36 deletions(-) diff --git a/airflow-core/tests/unit/core/test_logging_config.py b/airflow-core/tests/unit/core/test_logging_config.py index 60db2a80760a2..55bab2be66200 100644 --- a/airflow-core/tests/unit/core/test_logging_config.py +++ b/airflow-core/tests/unit/core/test_logging_config.py @@ -131,40 +131,9 @@ DEFAULT_REMOTE_CONN_ID = "test_conn_id" """ -SETTINGS_FILE_NESTED_MODULE = """ -LOGGING_CONFIG = { - 'version': 1, - 'disable_existing_loggers': False, - 'formatters': { - 'airflow.task': { - 'format': '[%(asctime)s] {%(filename)s:%(lineno)d} %(levelname)s - %(message)s' - }, - }, - 'handlers': { - 'console': { - 'class': 'logging.StreamHandler', - 'formatter': 'airflow.task', - 'stream': 'ext://sys.stdout' - }, - 'task': { - 'class': 'logging.StreamHandler', - 'formatter': 'airflow.task', - 'stream': 'ext://sys.stdout' - }, - }, - 'loggers': { - 'airflow.task': { - 'handlers': ['task'], - 'level': 'INFO', - 'propagate': False, - }, - } -} - -# Test remote logging variables -REMOTE_TASK_LOG = None -DEFAULT_REMOTE_CONN_ID = "nested_conn_id" -""" +SETTINGS_FILE_NESTED_MODULE = SETTINGS_FILE_SIMPLE_MODULE.replace( + 'DEFAULT_REMOTE_CONN_ID = "test_conn_id"', 'DEFAULT_REMOTE_CONN_ID = "nested_conn_id"' +) SETTINGS_FILE_NO_REMOTE_VARS = """ LOGGING_CONFIG = { @@ -293,7 +262,7 @@ def teardown_method(self): importlib.reload(airflow_local_settings) configure_logging() - def _verify_basic_logging_config(self, logging_config, logging_class_path, expected_path): + def _verify_basic_logging_config(self, logging_config: dict, logging_class_path: str, expected_path: str): """Helper method to verify basic logging config structure""" assert isinstance(logging_config, dict) assert logging_config["version"] == 1 @@ -486,7 +455,9 @@ def test_loading_remote_logging_with_hdfs_handler(self): ), ], ) - def test_load_logging_config_module_paths(self, settings_content, module_structure, expected_path): + def test_load_logging_config_module_paths( + self, settings_content: str, module_structure: str, expected_path: str + ): """Test that load_logging_config works with different module path structures""" dir_structure = module_structure.replace(".", "/") if module_structure else None From 2caedce8746c3584ebbed33365a716806328acc1 Mon Sep 17 00:00:00 2001 From: viiccwen Date: Sat, 16 Aug 2025 17:16:55 +0800 Subject: [PATCH 09/11] test(logging): use shared helper method for logging config validation Replace manual assertions with `self._verify_basic_logging_config` in fallback test to ensure consistent validation across all logging config tests. --- airflow-core/tests/unit/core/test_logging_config.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/airflow-core/tests/unit/core/test_logging_config.py b/airflow-core/tests/unit/core/test_logging_config.py index 55bab2be66200..79d852bf58a26 100644 --- a/airflow-core/tests/unit/core/test_logging_config.py +++ b/airflow-core/tests/unit/core/test_logging_config.py @@ -473,10 +473,11 @@ def test_load_logging_config_fallback_behavior(self): from airflow.logging_config import load_logging_config with patch("airflow.logging_config.log") as mock_log: - logging_config, _ = load_logging_config() + logging_config, logging_class_path = load_logging_config() - assert isinstance(logging_config, dict) - assert logging_config["version"] == 1 + self._verify_basic_logging_config( + logging_config, logging_class_path, f"{SETTINGS_DEFAULT_NAME}.LOGGING_CONFIG" + ) mock_log.info.assert_called_with( "Remote task logs will not be available due to an error: %s", From 23d891f6e6b1db5b506827c3da2ee927221b850d Mon Sep 17 00:00:00 2001 From: viiccwen Date: Sun, 17 Aug 2025 22:21:16 +0800 Subject: [PATCH 10/11] refactor(logging_config): simplify, improve test config vars - Remove redundant `SETTINGS_FILE_NESTED_MODULE` - Rename `SETTINGS_FILE_SIMPLE_MODULE` to `SETTINGS_FILE_WITH_REMOTE_VARS` --- airflow-core/src/airflow/logging_config.py | 3 +- .../tests/unit/core/test_logging_config.py | 75 ++----------------- 2 files changed, 10 insertions(+), 68 deletions(-) diff --git a/airflow-core/src/airflow/logging_config.py b/airflow-core/src/airflow/logging_config.py index 8ae7b50bc82c6..a391c1c6361a8 100644 --- a/airflow-core/src/airflow/logging_config.py +++ b/airflow-core/src/airflow/logging_config.py @@ -19,12 +19,13 @@ import logging import warnings +from importlib import import_module from logging.config import dictConfig from typing import TYPE_CHECKING, Any from airflow.configuration import conf from airflow.exceptions import AirflowConfigException -from airflow.utils.module_loading import import_module, import_string +from airflow.utils.module_loading import import_string if TYPE_CHECKING: from airflow.logging.remote import RemoteLogIO diff --git a/airflow-core/tests/unit/core/test_logging_config.py b/airflow-core/tests/unit/core/test_logging_config.py index 79d852bf58a26..369c8c54b4ef3 100644 --- a/airflow-core/tests/unit/core/test_logging_config.py +++ b/airflow-core/tests/unit/core/test_logging_config.py @@ -96,76 +96,17 @@ # Other settings here """ -SETTINGS_FILE_SIMPLE_MODULE = """ -LOGGING_CONFIG = { - 'version': 1, - 'disable_existing_loggers': False, - 'formatters': { - 'airflow.task': { - 'format': '[%(asctime)s] {%(filename)s:%(lineno)d} %(levelname)s - %(message)s' - }, - }, - 'handlers': { - 'console': { - 'class': 'logging.StreamHandler', - 'formatter': 'airflow.task', - 'stream': 'ext://sys.stdout' - }, - 'task': { - 'class': 'logging.StreamHandler', - 'formatter': 'airflow.task', - 'stream': 'ext://sys.stdout' - }, - }, - 'loggers': { - 'airflow.task': { - 'handlers': ['task'], - 'level': 'INFO', - 'propagate': False, - }, - } -} - -# Test remote logging variables +SETTINGS_FILE_WITH_REMOTE_VARS = ( + SETTINGS_FILE_VALID + + """ REMOTE_TASK_LOG = None DEFAULT_REMOTE_CONN_ID = "test_conn_id" """ - -SETTINGS_FILE_NESTED_MODULE = SETTINGS_FILE_SIMPLE_MODULE.replace( - 'DEFAULT_REMOTE_CONN_ID = "test_conn_id"', 'DEFAULT_REMOTE_CONN_ID = "nested_conn_id"' ) -SETTINGS_FILE_NO_REMOTE_VARS = """ -LOGGING_CONFIG = { - 'version': 1, - 'disable_existing_loggers': False, - 'formatters': { - 'airflow.task': { - 'format': '[%(asctime)s] {%(filename)s:%(lineno)d} %(levelname)s - %(message)s' - }, - }, - 'handlers': { - 'console': { - 'class': 'logging.StreamHandler', - 'formatter': 'airflow.task', - 'stream': 'ext://sys.stdout' - }, - 'task': { - 'class': 'logging.StreamHandler', - 'formatter': 'airflow.task', - 'stream': 'ext://sys.stdout' - }, - }, - 'loggers': { - 'airflow.task': { - 'handlers': ['task'], - 'level': 'INFO', - 'propagate': False, - }, - } -} -# Note: No REMOTE_TASK_LOG or DEFAULT_REMOTE_CONN_ID defined -""" +SETTINGS_FILE_NO_REMOTE_VARS = SETTINGS_FILE_WITH_REMOTE_VARS.replace("REMOTE_TASK_LOG = None", "").replace( + 'DEFAULT_REMOTE_CONN_ID = "test_conn_id"', "" +) SETTINGS_DEFAULT_NAME = "custom_airflow_local_settings" @@ -447,9 +388,9 @@ def test_loading_remote_logging_with_hdfs_handler(self): @pytest.mark.parametrize( "settings_content,module_structure,expected_path", [ - (SETTINGS_FILE_SIMPLE_MODULE, None, f"{SETTINGS_DEFAULT_NAME}.LOGGING_CONFIG"), + (SETTINGS_FILE_WITH_REMOTE_VARS, None, f"{SETTINGS_DEFAULT_NAME}.LOGGING_CONFIG"), ( - SETTINGS_FILE_NESTED_MODULE, + SETTINGS_FILE_WITH_REMOTE_VARS, "nested.config.module", f"nested.config.module.{SETTINGS_DEFAULT_NAME}.LOGGING_CONFIG", ), From 1be97583c56b40f650900bbcac16c3a7e68c380c Mon Sep 17 00:00:00 2001 From: viiccwen Date: Tue, 19 Aug 2025 18:02:03 +0800 Subject: [PATCH 11/11] refactor(logging): imporve & simplify logging testing file 1. remove `SETTING_FILE_NO_REMOTE_VARS` bcz redundant var 2. add return typo with `_verify_basic_logging_config` 3. add unittest.mock `call` --- .../tests/unit/core/test_logging_config.py | 26 ++++++++----------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/airflow-core/tests/unit/core/test_logging_config.py b/airflow-core/tests/unit/core/test_logging_config.py index 369c8c54b4ef3..b92a7f0bcfb0c 100644 --- a/airflow-core/tests/unit/core/test_logging_config.py +++ b/airflow-core/tests/unit/core/test_logging_config.py @@ -24,7 +24,7 @@ import pathlib import sys import tempfile -from unittest.mock import patch +from unittest.mock import call, patch import pytest @@ -96,17 +96,10 @@ # Other settings here """ -SETTINGS_FILE_WITH_REMOTE_VARS = ( - SETTINGS_FILE_VALID - + """ +SETTINGS_FILE_WITH_REMOTE_VARS = f"""{SETTINGS_FILE_VALID} REMOTE_TASK_LOG = None DEFAULT_REMOTE_CONN_ID = "test_conn_id" """ -) - -SETTINGS_FILE_NO_REMOTE_VARS = SETTINGS_FILE_WITH_REMOTE_VARS.replace("REMOTE_TASK_LOG = None", "").replace( - 'DEFAULT_REMOTE_CONN_ID = "test_conn_id"', "" -) SETTINGS_DEFAULT_NAME = "custom_airflow_local_settings" @@ -203,7 +196,9 @@ def teardown_method(self): importlib.reload(airflow_local_settings) configure_logging() - def _verify_basic_logging_config(self, logging_config: dict, logging_class_path: str, expected_path: str): + def _verify_basic_logging_config( + self, logging_config: dict, logging_class_path: str, expected_path: str + ) -> None: """Helper method to verify basic logging config structure""" assert isinstance(logging_config, dict) assert logging_config["version"] == 1 @@ -410,7 +405,7 @@ def test_load_logging_config_module_paths( def test_load_logging_config_fallback_behavior(self): """Test that load_logging_config falls back gracefully when remote logging vars are missing""" - with settings_context(SETTINGS_FILE_NO_REMOTE_VARS): + with settings_context(SETTINGS_FILE_VALID): from airflow.logging_config import load_logging_config with patch("airflow.logging_config.log") as mock_log: @@ -420,7 +415,8 @@ def test_load_logging_config_fallback_behavior(self): logging_config, logging_class_path, f"{SETTINGS_DEFAULT_NAME}.LOGGING_CONFIG" ) - mock_log.info.assert_called_with( - "Remote task logs will not be available due to an error: %s", - mock_log.info.call_args[0][1], - ) + mock_log.info.mock_calls = [ + call( + "Remote task logs will not be available due to an error: %s", + ) + ]