Skip to content

Commit

Permalink
Prevent registered model name from containing path separator (#7892)
Browse files Browse the repository at this point in the history
Signed-off-by: harupy <hkawamura0130@gmail.com>
  • Loading branch information
harupy committed Feb 25, 2023
1 parent 36c787a commit 63ef72a
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 1 deletion.
12 changes: 11 additions & 1 deletion mlflow/store/model_registry/file_store.py
Expand Up @@ -37,7 +37,7 @@
from mlflow.utils.validation import (
_validate_registered_model_tag,
_validate_model_version_tag,
_validate_model_name,
_validate_model_name as _original_validate_model_name,
_validate_model_version,
_validate_tag_name,
)
Expand All @@ -56,6 +56,7 @@
make_containing_dirs,
list_all,
local_file_uri_to_path,
contains_path_separator,
)
from mlflow.utils.time_utils import get_current_time_millis

Expand All @@ -67,6 +68,15 @@ def _default_root_dir():
return get_env(_REGISTRY_DIR_ENV_VAR) or os.path.abspath(DEFAULT_LOCAL_FILE_AND_ARTIFACT_PATH)


def _validate_model_name(name):
_original_validate_model_name(name)
if contains_path_separator(name):
raise MlflowException(
f"Invalid name: '{name}'. Registered model name cannot contain path separator",
INVALID_PARAMETER_VALUE,
)


class FileStore(AbstractStore):
MODELS_FOLDER_NAME = "models"
META_DATA_FILE_NAME = "meta.yaml"
Expand Down
7 changes: 7 additions & 0 deletions mlflow/utils/file_utils.py
Expand Up @@ -711,3 +711,10 @@ def shutil_copytree_without_file_permissions(src_dir, dst_dir):
relative_file_path = os.path.relpath(file_path, src_dir)
abs_file_path = os.path.join(dst_dir, relative_file_path)
shutil.copyfile(file_path, abs_file_path)


def contains_path_separator(path):
"""
Returns True if a path contains a path separator, False otherwise.
"""
return any((sep in path) for sep in (os.path.sep, os.path.altsep) if sep is not None)
8 changes: 8 additions & 0 deletions tests/store/model_registry/test_file_store.py
Expand Up @@ -62,6 +62,14 @@ def test_create_registered_model(store):
assert model.tags == {}


def test_create_registered_model_with_name_that_looks_like_path(store, tmp_path):
name = str(tmp_path.joinpath("test"))
with pytest.raises(
MlflowException, match=r"Registered model name cannot contain path separator"
):
store.get_registered_model(name)


def _verify_registered_model(fs, name, rm_data):
rm = fs.get_registered_model(name)
assert rm.name == name
Expand Down

0 comments on commit 63ef72a

Please sign in to comment.