[to #45773874]fix: get_model revision=None bug, and hub case occasionally delete test model failed

Link: https://code.alibaba-inc.com/Ali-MaaS/MaaS-lib/codereview/10549680

modelscope/hub/api.py

@@ -176,7 +176,10 @@ class HubApi:
         """
         cookies = ModelScopeConfig.get_cookies()
         owner_or_group, name = model_id_to_group_owner_name(model_id)
-        path = f'{self.endpoint}/api/v1/models/{owner_or_group}/{name}?Revision={revision}'
+        if revision:
+            path = f'{self.endpoint}/api/v1/models/{owner_or_group}/{name}?Revision={revision}'
+        else:
+            path = f'{self.endpoint}/api/v1/models/{owner_or_group}/{name}'
 
         r = requests.get(path, cookies=cookies, headers=self.headers)
         handle_http_response(r, logger, cookies, model_id)
@@ -447,8 +450,12 @@ class HubApi:
         Returns:
             List[dict]: Model file list.
         """
-        path = '%s/api/v1/models/%s/repo/files?Revision=%s&Recursive=%s' % (
-            self.endpoint, model_id, revision, recursive)
+        if revision:
+            path = '%s/api/v1/models/%s/repo/files?Revision=%s&Recursive=%s' % (
+                self.endpoint, model_id, revision, recursive)
+        else:
+            path = '%s/api/v1/models/%s/repo/files?Recursive=%s' % (
+                self.endpoint, model_id, recursive)
         cookies = self._check_cookie(use_cookies)
         if root is not None:
             path = path + f'&Root={root}'

modelscope/hub/errors.py

@@ -63,6 +63,7 @@ def handle_http_post_error(response, url, request_body):
     except HTTPError as error:
         logger.error('Request %s with body: %s exception' %
                      (url, request_body))
+        logger.error('Response details: %s' % response.content)
         raise error
 
 

tests/hub/test_hub_operation.py

@@ -87,21 +87,23 @@ class HubOperationTest(unittest.TestCase):
         assert mdtime1 == mdtime2
 
     def test_download_public_without_login(self):
-        self.prepare_case()
-        rmtree(ModelScopeConfig.path_credential)
-        snapshot_path = snapshot_download(
-            model_id=self.model_id, revision=self.revision)
-        downloaded_file_path = os.path.join(snapshot_path,
-                                            download_model_file_name)
-        assert os.path.exists(downloaded_file_path)
-        temporary_dir = tempfile.mkdtemp()
-        downloaded_file = model_file_download(
-            model_id=self.model_id,
-            file_path=download_model_file_name,
-            revision=self.revision,
-            cache_dir=temporary_dir)
-        assert os.path.exists(downloaded_file)
-        self.api.login(TEST_ACCESS_TOKEN1)
+        try:
+            self.prepare_case()
+            rmtree(ModelScopeConfig.path_credential)
+            snapshot_path = snapshot_download(
+                model_id=self.model_id, revision=self.revision)
+            downloaded_file_path = os.path.join(snapshot_path,
+                                                download_model_file_name)
+            assert os.path.exists(downloaded_file_path)
+            temporary_dir = tempfile.mkdtemp()
+            downloaded_file = model_file_download(
+                model_id=self.model_id,
+                file_path=download_model_file_name,
+                revision=self.revision,
+                cache_dir=temporary_dir)
+            assert os.path.exists(downloaded_file)
+        finally:
+            self.api.login(TEST_ACCESS_TOKEN1)
 
     def test_snapshot_delete_download_cache_file(self):
         self.prepare_case()