fix(workflow): resolve FileLock path from file URI correctly - #2337
Open
Misty_Cirque (Cyber-Marty) wants to merge 1 commit into
Open
fix(workflow): resolve FileLock path from file URI correctly#2337Misty_Cirque (Cyber-Marty) wants to merge 1 commit into
Misty_Cirque (Cyber-Marty) wants to merge 1 commit into
Conversation
MLflowExpManager._get_or_create_exp built the FileLock path with
os.path.join(pr.netloc, pr.path.lstrip('/'), 'filelock'). For absolute
file URIs (including the default URI QSettings generates), lstrip('/')
stripped the leading slash and turned the lock path CWD-relative, so
the lock no longer serialized runs sharing the same URI from different
working directories. On Windows, drive-letter URIs (file:///C:/...)
only worked by accident under NT path semantics.
Build the lock path with urllib.request.url2pathname(pr.path) instead,
which converts the URI path to a filesystem path correctly on all
platforms and keeps relative URIs (file:mlruns) relative as before.
Fixes microsoft#2252 (primary defect; the _log_uncommitted_code issues in the
same report will be addressed separately).
Author
|
@microsoft-github-policy-service agree |
This was referenced Sep 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
MLflowExpManager._get_or_create_exp (qlib/workflow/expm.py:236) built its FileLock path as:
python
Path(os.path.join(pr.netloc, pr.path.lstrip("/"), "filelock"))
For any absolute file:// URI — including the default URI that qlib.config.QSettings constructs ("file:" + str(Path(os.getcwd()).resolve() / "mlruns")) — urlparse returns netloc="" and path="/absolute/...", so lstrip("/") turns the absolute path into a CWD-relative one. The lock then points under whatever directory the process happens to be in instead of the URI's location, and no longer serializes runs that share the same URI from different working directories.
On Windows, file:///C:/... drive-letter URIs only kept working by accident: after lstrip("/") the remaining C:/... is still absolute under NT path semantics, but the construction is fragile.
Fix
Build the lock path with urllib.request.url2pathname(pr.path), which converts a URI path to a filesystem path correctly on every platform (including Windows drive letters) and leaves relative URIs (file:mlruns) relative, preserving today's behavior for that shape.
URI shape: file:/Users/me/mlruns
before (POSIX): Users/me/mlruns/filelock (relative, CWD-dependent)
after (POSIX): /Users/me/mlruns/filelock
after (Windows): —
────────────────────────────────────────
URI shape: file:///Users/me/mlruns
before (POSIX): Users/me/mlruns/filelock (relative)
after (POSIX): /Users/me/mlruns/filelock
after (Windows): —
────────────────────────────────────────
URI shape: file:///C:/Users/me/mlruns
before (POSIX): C:/Users/me/mlruns/filelock (absolute by accident)
after (POSIX): /C:/Users/me/mlruns/filelock*
after (Windows): C:\Users\me\mlruns\filelock
────────────────────────────────────────
URI shape: file:mlruns
before (POSIX): mlruns/filelock
after (POSIX): unchanged
after (Windows): unchanged
*on POSIX the drive-letter form resolves as an absolute POSIX path; on Windows url2pathname produces the correct drive path (verified in the new tests).
Tests
New tests/workflow_tests/test_exp_manager_lock_path.py:
All new and existing workflow tests pass locally (tests/test_workflow.py + new file, 5 passed).
Fixes #2252 (primary defect). The _log_uncommitted_code stderr/shell=True issues from the same report will be addressed in a follow-up PR.