Skip to content

Fix MLflow CWD-sensitive workflow paths#2260

Open
Kevin-Li-2025 wants to merge 1 commit into
microsoft:mainfrom
Kevin-Li-2025:kevin/fix-mlflow-cwd-safety
Open

Fix MLflow CWD-sensitive workflow paths#2260
Kevin-Li-2025 wants to merge 1 commit into
microsoft:mainfrom
Kevin-Li-2025:kevin/fix-mlflow-cwd-safety

Conversation

@Kevin-Li-2025

Copy link
Copy Markdown

Summary

  • resolve file:// MLflow tracking URIs with url2pathname before building the experiment FileLock path, so absolute tracking paths stay absolute regardless of the caller CWD
  • skip optional uncommitted-code artifact logging when the process CWD is not a git work tree
  • remove shell=True from the git diff/status calls and capture stderr so git diagnostics do not leak into caller stderr
  • add regression coverage for absolute file URI lock paths and quiet non-git CWD handling

Fixes #2252.

Test plan

  • MLFLOW_ALLOW_FILE_STORE=true .venv/bin/python -m pytest tests/dependency_tests/test_mlflow.py -q
  • .venv/bin/python -m black qlib/workflow/expm.py qlib/workflow/recorder.py tests/dependency_tests/test_mlflow.py -l 120 --check --diff
  • .venv/bin/python -m pylint --disable=C0104,C0114,C0115,C0116,C0301,C0302,C0411,C0413,C1802,R0401,R0801,R0902,R0903,R0911,R0912,R0913,R0914,R0915,R0917,R1720,W0105,W0123,W0201,W0511,W0613,W1113,W1514,W4904,E0401,E1121,C0103,C0209,R0402,R1705,R1710,R1725,R1730,R1735,W0102,W0212,W0221,W0223,W0231,W0237,W0612,W0621,W0622,W0703,W1309,E1102,E1136 --const-rgx='[a-z_][a-z0-9_]{2,30}' qlib/workflow/expm.py qlib/workflow/recorder.py
  • .venv/bin/python -m flake8 --ignore=E501,F541,E266,E402,W503,E731,E203 --per-file-ignores="init.py:F401,F403" qlib

@Kevin-Li-2025

Copy link
Copy Markdown
Author

Small follow-up on this one: the change is scoped to making workflow artifact paths stable when callers change CWD after recorder initialization. The goal is to keep MLflow/FileLock paths anchored to the recorder/experiment URI rather than whichever directory happens to be current at log time.

CLA is green and the branch is mergeable. If there is a preferred Qlib workflow test location for this path-safety behavior, I can move or reshape the regression coverage accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

qlib.workflow: FileLock and _log_uncommitted_code both unsafe vs CWD

1 participant