Skip to content

[HWORKS-2064] Auto-increment model export only looks at the model version folder, should also account for model version db metadata #503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions python/hopsworks_common/core/dataset_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ def list_files(self, path, offset, limit):
return inode_lst["count"], inode.Inode.from_response_json(inode_lst)

@usage.method_logger
def list(self, remote_path, sort_by=None, limit=1000):
def list(self, remote_path, sort_by=None, offset=0, limit=1000):
"""List all files in a directory in datasets.

:param remote_path: path to list
Expand All @@ -659,7 +659,7 @@ def list(self, remote_path, sort_by=None, limit=1000):
# they seem to handle paths differently and return different results, which prevents the merge at the moment (2024-09-03), due to the requirement of backwards-compatibility
_client = client.get_instance()
path_params = ["project", _client._project_id, "dataset", remote_path]
query_params = {"action": "listing", "sort_by": sort_by, "limit": limit}
query_params = {"action": "listing", "sort_by": sort_by, "limit": limit, "offset": offset}
headers = {"content-type": "application/json"}
return _client._send_request(
"GET", path_params, headers=headers, query_params=query_params
Expand Down
66 changes: 50 additions & 16 deletions python/hsml/engine/model_engine.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,10 +270,17 @@ def _set_model_version(
# Set model version if not defined
if model_instance._version is None:
current_highest_version = 0
for item in self._dataset_api.list(dataset_model_path, sort_by="NAME:desc")[
"items"
]:
files = []
offset = 0
limit = 1000
items = self._dataset_api.list(dataset_model_path, sort_by="NAME:desc", offset=offset, limit=limit)["items"]
while items:
files = files + items
offset += limit
items = self._dataset_api.list(dataset_model_path, sort_by="NAME:desc", offset=offset, limit=limit)["items"]
for item in files:
_, file_name = os.path.split(item["attributes"]["path"])
# Get highest version folder
try:
try:
current_version = int(file_name)
Expand All @@ -283,20 +290,47 @@ def _set_model_version(
current_highest_version = current_version
except RestAPIError:
pass
model_instance._version = current_highest_version + 1

elif self._dataset_api.path_exists(
dataset_models_root_path
+ "/"
+ model_instance._name
+ "/"
+ str(model_instance._version)
):
raise ModelRegistryException(
"Model with name {} and version {} already exists".format(
model_instance._name, model_instance._version

current_highest_version += 1

# Get highest available model metadata version
# This makes sure we skip corrupt versions where the model folder is deleted manually but the backend metadata is still there
model = self._model_api.get(model_instance._name, current_highest_version, model_instance.model_registry_id)
while model:
current_highest_version += 1
model = self._model_api.get(model_instance._name, current_highest_version, model_instance.model_registry_id)

model_instance._version = current_highest_version
else:
model_backend_object_exists = self._model_api.get(model_instance._name, model_instance._version, model_instance.model_registry_id) is not None
model_version_folder_exists = self._dataset_api.path_exists(
dataset_models_root_path
+ "/"
+ model_instance._name
+ "/"
+ str(model_instance._version)
)

# Perform validations to handle possible inconsistency between db and filesystem
if model_backend_object_exists and not model_version_folder_exists:
raise ModelRegistryException(
"Model with name {0} and version {1} looks to be corrupt as the version is registered but there is no Models/{0}/{1} folder in the filesystem. Delete this version using Model.delete() or the UI and try to export this version again.".format(
model_instance._name, model_instance._version
)
)
)
elif not model_backend_object_exists and model_version_folder_exists:
raise ModelRegistryException(
"Model with name {0} and version {1} looks to be corrupt as the version exists in the filesystem but it is not registered. To proceed, please delete the Models/{0}/{1} folder manually and try to save this model again.".format(
model_instance._name, model_instance._version
)
)
elif model_backend_object_exists and model_version_folder_exists:
raise ModelRegistryException(
"Model with name {} and version {} already exists, please select another version.".format(
model_instance._name, model_instance._version
)
)

return model_instance

def _build_resource_path(self, model_instance, artifact):
Expand Down
Loading