Skip to content

Commit 462ce9a

Browse files
authored
Fix module loading in logging config (#54555)
* 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 * 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 * 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 * 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 * 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 * 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 * refactor(logging): move `import_module` to the top level * test(logging): optimize test constants to reduce code duplication Replace `SETTINGS_FILE_NESTED_MODULE` replicate part with string replacement from `SETTINGS_FILE_SIMPLE_MODULE` * 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. * 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` * 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`
1 parent 5484300 commit 462ce9a

File tree

2 files changed

+60
-2
lines changed

2 files changed

+60
-2
lines changed

airflow-core/src/airflow/logging_config.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919

2020
import logging
2121
import warnings
22+
from importlib import import_module
2223
from logging.config import dictConfig
2324
from typing import TYPE_CHECKING, Any
2425

@@ -73,7 +74,9 @@ def load_logging_config() -> tuple[dict[str, Any], str]:
7374
else:
7475
modpath = logging_class_path.rsplit(".", 1)[0]
7576
try:
76-
mod = import_string(modpath)
77+
mod = import_module(modpath)
78+
79+
# Load remote logging configuration from the custom module
7780
REMOTE_TASK_LOG = getattr(mod, "REMOTE_TASK_LOG")
7881
DEFAULT_REMOTE_CONN_ID = getattr(mod, "DEFAULT_REMOTE_CONN_ID", None)
7982
except Exception as err:

airflow-core/tests/unit/core/test_logging_config.py

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
import pathlib
2525
import sys
2626
import tempfile
27-
from unittest.mock import patch
27+
from unittest.mock import call, patch
2828

2929
import pytest
3030

@@ -96,6 +96,11 @@
9696
# Other settings here
9797
"""
9898

99+
SETTINGS_FILE_WITH_REMOTE_VARS = f"""{SETTINGS_FILE_VALID}
100+
REMOTE_TASK_LOG = None
101+
DEFAULT_REMOTE_CONN_ID = "test_conn_id"
102+
"""
103+
99104
SETTINGS_DEFAULT_NAME = "custom_airflow_local_settings"
100105

101106

@@ -191,6 +196,15 @@ def teardown_method(self):
191196
importlib.reload(airflow_local_settings)
192197
configure_logging()
193198

199+
def _verify_basic_logging_config(
200+
self, logging_config: dict, logging_class_path: str, expected_path: str
201+
) -> None:
202+
"""Helper method to verify basic logging config structure"""
203+
assert isinstance(logging_config, dict)
204+
assert logging_config["version"] == 1
205+
assert "airflow.task" in logging_config["loggers"]
206+
assert logging_class_path == expected_path
207+
194208
# When we try to load an invalid config file, we expect an error
195209
def test_loading_invalid_local_settings(self):
196210
from airflow.logging_config import configure_logging, log
@@ -365,3 +379,44 @@ def test_loading_remote_logging_with_hdfs_handler(self):
365379
airflow.logging_config.configure_logging()
366380

367381
assert isinstance(airflow.logging_config.REMOTE_TASK_LOG, HdfsRemoteLogIO)
382+
383+
@pytest.mark.parametrize(
384+
"settings_content,module_structure,expected_path",
385+
[
386+
(SETTINGS_FILE_WITH_REMOTE_VARS, None, f"{SETTINGS_DEFAULT_NAME}.LOGGING_CONFIG"),
387+
(
388+
SETTINGS_FILE_WITH_REMOTE_VARS,
389+
"nested.config.module",
390+
f"nested.config.module.{SETTINGS_DEFAULT_NAME}.LOGGING_CONFIG",
391+
),
392+
],
393+
)
394+
def test_load_logging_config_module_paths(
395+
self, settings_content: str, module_structure: str, expected_path: str
396+
):
397+
"""Test that load_logging_config works with different module path structures"""
398+
dir_structure = module_structure.replace(".", "/") if module_structure else None
399+
400+
with settings_context(settings_content, dir_structure):
401+
from airflow.logging_config import load_logging_config
402+
403+
logging_config, logging_class_path = load_logging_config()
404+
self._verify_basic_logging_config(logging_config, logging_class_path, expected_path)
405+
406+
def test_load_logging_config_fallback_behavior(self):
407+
"""Test that load_logging_config falls back gracefully when remote logging vars are missing"""
408+
with settings_context(SETTINGS_FILE_VALID):
409+
from airflow.logging_config import load_logging_config
410+
411+
with patch("airflow.logging_config.log") as mock_log:
412+
logging_config, logging_class_path = load_logging_config()
413+
414+
self._verify_basic_logging_config(
415+
logging_config, logging_class_path, f"{SETTINGS_DEFAULT_NAME}.LOGGING_CONFIG"
416+
)
417+
418+
mock_log.info.mock_calls = [
419+
call(
420+
"Remote task logs will not be available due to an error: %s",
421+
)
422+
]

0 commit comments

Comments
 (0)