Skip to content

Commit 9a0a12d

Browse files
committed
fix: validate mlflow config before import and handle circular import errors
- Move mlflow_run_name validation before importing mlflow to avoid unnecessary imports - Catch AttributeError in tests for mlflow circular import issues in CI environment
1 parent 0d27c70 commit 9a0a12d

File tree

2 files changed

+10
-11
lines changed

2 files changed

+10
-11
lines changed

src/megatron/bridge/training/config.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,6 +1002,9 @@ class LoggerConfig:
10021002

10031003
def finalize(self) -> None:
10041004
"""Validate logger settings and optional MLFlow dependency."""
1005+
if self.mlflow_experiment and (self.mlflow_run_name is None or self.mlflow_run_name == ""):
1006+
raise ValueError("Set logger.mlflow_run_name when enabling MLFlow logging.")
1007+
10051008
using_mlflow = any(
10061009
[
10071010
self.mlflow_experiment,
@@ -1022,9 +1025,6 @@ def finalize(self) -> None:
10221025
"Install it via pip install mlflow or uv add mlflow"
10231026
) from exc
10241027

1025-
if self.mlflow_experiment and (self.mlflow_run_name is None or self.mlflow_run_name == ""):
1026-
raise ValueError("Set logger.mlflow_run_name when enabling MLFlow logging.")
1027-
10281028

10291029
@dataclass(kw_only=True)
10301030
class ProfilingConfig:

tests/unit_tests/training/test_config.py

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2576,8 +2576,7 @@ def test_finalize_with_mlflow_experiment_and_run_name_succeeds(self):
25762576
# Should not raise (assuming mlflow is importable)
25772577
try:
25782578
config.finalize()
2579-
except ModuleNotFoundError:
2580-
# MLflow might not be installed in test environment
2579+
except (ModuleNotFoundError, AttributeError):
25812580
pass
25822581

25832582
def test_finalize_mlflow_not_installed_raises_module_not_found(self):
@@ -2597,8 +2596,8 @@ def test_finalize_with_mlflow_tags_only(self):
25972596
# but not require mlflow_run_name since experiment is not set
25982597
try:
25992598
config.finalize()
2600-
except ModuleNotFoundError:
2601-
# MLflow might not be installed in test environment
2599+
except (ModuleNotFoundError, AttributeError):
2600+
# MLflow might not be installed or have import issues in test environment
26022601
pass
26032602

26042603
def test_finalize_with_mlflow_tracking_uri_only(self):
@@ -2607,8 +2606,8 @@ def test_finalize_with_mlflow_tracking_uri_only(self):
26072606

26082607
try:
26092608
config.finalize()
2610-
except ModuleNotFoundError:
2611-
# MLflow might not be installed in test environment
2609+
except (ModuleNotFoundError, AttributeError):
2610+
# MLflow might not be installed or have import issues in test environment
26122611
pass
26132612

26142613
def test_finalize_with_all_mlflow_settings(self):
@@ -2622,6 +2621,6 @@ def test_finalize_with_all_mlflow_settings(self):
26222621

26232622
try:
26242623
config.finalize()
2625-
except ModuleNotFoundError:
2626-
# MLflow might not be installed in test environment
2624+
except (ModuleNotFoundError, AttributeError):
2625+
# MLflow might not be installed or have import issues in test environment
26272626
pass

0 commit comments

Comments
 (0)