Skip to content

Commit a38dd81

Browse files
fix: address CodeRabbit review findings
- mlflow_backend.py: set mlflow.runName tag explicitly when resuming a run with run_id, since some MLflow versions ignore run_name in start_run() during resume - export.py: wrap MLFLOW_WORKSPACE restoration in finally block so it executes even if set_experiment/start_run raises - export.py: validate tracking_uri with assert_tracking_uri_has_no_userinfo() before composing the Slack-visible MLflow URL Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 0616bac commit a38dd81

2 files changed

Lines changed: 27 additions & 19 deletions

File tree

projects/caliper/engine/file_export/mlflow_backend.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,8 @@ def _run(uri: str | None) -> tuple[str, dict[str, Any] | None]:
423423
client = mlflow.tracking.MlflowClient()
424424
with mlflow.start_run(**start_kw):
425425
rid = mlflow.active_run().info.run_id
426+
if run_id and run_name:
427+
mlflow.set_tag("mlflow.runName", run_name)
426428
_apply_run_metadata(effective_meta)
427429
_apply_log_model(artifact_root, effective_meta, verbose=verbose)
428430

@@ -560,6 +562,8 @@ def _run(uri: str | None) -> tuple[str, dict[str, Any] | None]:
560562

561563
with mlflow.start_run(**start_kw) as parent:
562564
parent_rid = parent.info.run_id
565+
if run_id and parent_run_name:
566+
mlflow.set_tag("mlflow.runName", parent_run_name)
563567
_apply_run_metadata(effective_meta)
564568

565569
_upload_mlflow_files_parallel(

projects/caliper/orchestration/export.py

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -308,24 +308,25 @@ def precreate_mlflow_run() -> dict[str, str]:
308308
tracking_uri = secrets_data.get("tracking_uri", "")
309309

310310
prev_workspace = os.environ.get("MLFLOW_WORKSPACE")
311-
with mlflow_connection_env(secrets_data):
312-
if tracking_uri:
313-
mlflow.set_tracking_uri(tracking_uri)
314-
if workspace:
315-
os.environ["MLFLOW_WORKSPACE"] = workspace
316-
if experiment:
317-
mlflow.set_experiment(experiment)
318-
319-
run_name = os.environ.get("FJOB_NAME")
320-
with mlflow.start_run(run_name=run_name):
321-
active = mlflow.active_run()
322-
run_id = active.info.run_id
323-
experiment_id = str(active.info.experiment_id)
324-
325-
if prev_workspace is not None:
326-
os.environ["MLFLOW_WORKSPACE"] = prev_workspace
327-
else:
328-
os.environ.pop("MLFLOW_WORKSPACE", None)
311+
try:
312+
with mlflow_connection_env(secrets_data):
313+
if tracking_uri:
314+
mlflow.set_tracking_uri(tracking_uri)
315+
if workspace:
316+
os.environ["MLFLOW_WORKSPACE"] = workspace
317+
if experiment:
318+
mlflow.set_experiment(experiment)
319+
320+
run_name = os.environ.get("FJOB_NAME")
321+
with mlflow.start_run(run_name=run_name):
322+
active = mlflow.active_run()
323+
run_id = active.info.run_id
324+
experiment_id = str(active.info.experiment_id)
325+
finally:
326+
if prev_workspace is not None:
327+
os.environ["MLFLOW_WORKSPACE"] = prev_workspace
328+
else:
329+
os.environ.pop("MLFLOW_WORKSPACE", None)
329330

330331
logger.info("Pre-created MLflow run %s (experiment=%s)", run_id, experiment_id)
331332

@@ -364,7 +365,9 @@ def _read_mlflow_ids_from_marker() -> tuple[str, str]:
364365
def build_mlflow_run_url() -> str:
365366
"""Construct the MLflow run URL at runtime from vault secrets and the marker file."""
366367
try:
367-
from projects.caliper.engine.file_export.mlflow_secrets import load_mlflow_secrets_yaml
368+
from projects.caliper.engine.file_export.mlflow_secrets import (
369+
load_mlflow_secrets_yaml,
370+
)
368371
from projects.core.library import config
369372

370373
run_id, experiment_id = _read_mlflow_ids_from_marker()
@@ -391,6 +394,7 @@ def build_mlflow_run_url() -> str:
391394
tracking_uri = secrets_data.get("tracking_uri", "").rstrip("/")
392395
if not tracking_uri.startswith(("http://", "https://")):
393396
return ""
397+
assert_tracking_uri_has_no_userinfo(tracking_uri)
394398

395399
workspace = config.project.get_config(
396400
"caliper.export.backend.mlflow.config.workspace", None, print=False, warn=False

0 commit comments

Comments
 (0)