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

Author: robzor92

python/hopsworks_common/core/dataset_api.py

@@ -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
@@ -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

python/hsml/engine/model_engine.py

@@ -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)
@@ -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):