Create parent directories for SQLite database files#19205
Create parent directories for SQLite database files#19205harupy merged 1 commit intomlflow:masterfrom
Conversation
Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
2e396ee to
e0d7525
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds automatic creation of parent directories for SQLite database files when using nested paths in database URIs like sqlite:///path/to/nested/dir/mlflow.db. The logic is integrated into create_sqlalchemy_engine_with_retry to apply to all SQLite connections.
Key Changes:
- Added
_make_parent_dirs_if_sqlite()helper function to create parent directories for SQLite database files - Integrated the helper into
create_sqlalchemy_engine_with_retry()to automatically create directories before engine creation - Added unit tests covering nested paths, in-memory databases, and non-SQLite URIs
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| mlflow/store/db/utils.py | Adds _make_parent_dirs_if_sqlite() function and integrates it into create_sqlalchemy_engine_with_retry() to create parent directories before SQLite database creation |
| tests/store/db/test_utils.py | Adds three test cases covering the main functionality (nested directory creation), in-memory database handling, and non-SQLite URI handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def _make_parent_dirs_if_sqlite(db_uri: str) -> None: | ||
| """Create parent directories for SQLite database file if they don't exist.""" | ||
| if not db_uri.startswith("sqlite:///"): |
There was a problem hiding this comment.
The function only checks for sqlite:/// (three slashes), which is the Unix/Linux format. On Windows, SQLite URIs use sqlite:// (two slashes) for absolute paths, as seen in mlflow/utils/file_utils.py:493. This means the function will not create parent directories for SQLite databases on Windows.
Consider using sqlite: as the prefix check and properly parsing the path based on the platform, or use a more robust URI parsing approach like:
if not db_uri.startswith("sqlite:"):
return
db_path = db_uri.removeprefix("sqlite://")
# Remove leading slash for proper path handling
db_path = db_path.lstrip("/")|
Documentation preview for e0d7525 is available at: More info
|
Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
Signed-off-by: harupy <17039389+harupy@users.noreply.github.com>
🛠 DevTools 🛠
Install mlflow from this PR
For Databricks, use the following command:
What changes are proposed in this pull request?
When using a SQLite URI like
sqlite:///path/to/nested/dir/mlflow.db, the parent directories are now automatically created if they don't exist. This prevents errors when the user specifies a path to a non-existent directory.The logic is added to
create_sqlalchemy_engine_with_retryso all SQLite database connections benefit from this fix automatically.In-memory databases (
:memory:) and empty paths are correctly skipped.How is this PR tested?
Does this PR require documentation update?
Release Notes
Is this a user-facing change?
Automatically create parent directories for SQLite database files when they don't exist.
What component(s), interfaces, languages, and integrations does this PR affect?
Components
area/tracking: Tracking Service, tracking client APIs, autologgingarea/model-registry: Model Registry service, APIs, and the fluent client calls for Model RegistryHow should the PR be classified in the release notes? Choose one:
rn/feature- A new user-facing feature worth mentioning in the release notesShould this PR be included in the next patch release?
🤖 Generated with Claude Code