Merge pull request #27 from modelscope/fix/v9_reviews

[Fix] Fix review issues

Author: wangxingjun778

pyproject.toml

@@ -45,6 +45,9 @@ build-backend = "setuptools.build_meta"
 [tool.setuptools.packages.find]
 where = ["src"]
 
+[tool.setuptools.package-data]
+modelscope_hub = ["py.typed"]
+
 [tool.setuptools.dynamic]
 version = {attr = "modelscope_hub.version.__version__"}
 

src/modelscope_hub/_cache_manager.py

@@ -86,6 +86,40 @@ def scan_cache(cache_dir: Path | None = None) -> CacheInfo:
                 local_path=str(repo_dir),
             ))
 
+    # Scan flat layout (compat): {root}/{owner}/{name}/
+    # and legacy layout: {root}/hub/{owner}/{name}/
+    _scanned_paths = {r.local_path for r in repos}  # avoid double-counting
+
+    for prefix, prefix_path in [("", root), ("hub", root / "hub")]:
+        if not prefix_path.is_dir():
+            continue
+        for owner_dir in prefix_path.iterdir():
+            if not owner_dir.is_dir():
+                continue
+            # Skip known non-repo directories
+            if owner_dir.name in ("hub", "models", "datasets", "studios", "mcps", "skills"):
+                continue
+            for name_dir in owner_dir.iterdir():
+                if not name_dir.is_dir():
+                    continue
+                if str(name_dir) in _scanned_paths:
+                    continue
+
+                size = _dir_size(name_dir)
+                total_size += size
+                nb_files = sum(1 for f in name_dir.rglob("*") if f.is_file())
+                repo_id = f"{owner_dir.name}/{name_dir.name}"
+
+                repos.append(CachedRepoInfo(
+                    repo_id=repo_id,
+                    repo_type=RepoType.MODEL,  # assume model for flat layout
+                    revision=None,
+                    size_on_disk=size,
+                    nb_files=nb_files,
+                    last_accessed=None,
+                    local_path=str(name_dir),
+                ))
+
     return CacheInfo(
         repos=repos,
         total_size=total_size,
@@ -135,30 +169,42 @@ def clear_cache(
     freed = 0
 
     if repo_id and repo_type:
-        # Clear specific repo
-        segment = f"{repo_type}s" if not repo_type.endswith("s") else repo_type
-        safe_id = repo_id.replace("/", "--")
-        target = root / segment / safe_id
-        if target.is_dir():
-            freed = _dir_size(target)
+        # Clear specific repo — check all possible layout locations
+        targets = _resolve_cache_targets(root, repo_id, repo_type)
+        for target in targets:
+            size = _dir_size(target)
+            freed += size
             _safe_rmtree(target)
-            logger.info("Cleared cache for %s/%s (%d bytes)", repo_type, repo_id, freed)
+            logger.info("Cleared cache at %s (%d bytes)", target, size)
     elif repo_type:
-        # Clear all repos of this type
+        # Clear all repos of this type (standard layout)
         segment = f"{repo_type}s" if not repo_type.endswith("s") else repo_type
         type_dir = root / segment
         if type_dir.is_dir():
-            freed = _dir_size(type_dir)
+            size = _dir_size(type_dir)
+            freed += size
             _safe_rmtree(type_dir)
-            logger.info("Cleared all %s caches (%d bytes)", repo_type, freed)
+            logger.info("Cleared %s standard cache (%d bytes)", repo_type, size)
+        # Also clear legacy hub layout
+        hub_dir = root / "hub"
+        if hub_dir.is_dir():
+            size = _dir_size(hub_dir)
+            freed += size
+            _safe_rmtree(hub_dir)
+            logger.info("Cleared legacy hub cache (%d bytes)", size)
     else:
-        # Clear everything
+        # Clear everything (standard + legacy layouts)
         for repo_t in _DEFAULT_SCAN_TYPES:
             segment = f"{repo_t}s"
             type_dir = root / segment
             if type_dir.is_dir():
                 freed += _dir_size(type_dir)
                 _safe_rmtree(type_dir)
+        # Legacy hub directory
+        hub_dir = root / "hub"
+        if hub_dir.is_dir():
+            freed += _dir_size(hub_dir)
+            _safe_rmtree(hub_dir)
         logger.info("Cleared all caches (%d bytes)", freed)
 
     return freed
@@ -167,6 +213,25 @@ def clear_cache(
 # ---------------------------------------------------------------------------
 # Helpers
 # ---------------------------------------------------------------------------
+def _resolve_cache_targets(root: Path, repo_id: str, repo_type: str) -> list[Path]:
+    """Resolve all possible cache locations for a repo across layout formats.
+
+    Checks three path layouts:
+    - Standard:  {root}/{type}s/{owner}--{name}/
+    - Flat:      {root}/{owner}/{name}/
+    - Legacy:    {root}/hub/{owner}/{name}/
+    """
+    safe_id = repo_id.replace("/", "--")
+    segment = f"{repo_type}s" if not repo_type.endswith("s") else repo_type
+
+    candidates = [
+        root / segment / safe_id,    # standard: {cache}/models/owner--name/
+        root / repo_id,              # flat (compat): {cache}/owner/name/
+        root / "hub" / repo_id,      # legacy: {cache}/hub/owner/name/
+    ]
+    return [p for p in candidates if p.is_dir()]
+
+
 def _dir_size(path: Path) -> int:
     """Compute total size of all files under ``path`` recursively."""
     total = 0

src/modelscope_hub/api.py

@@ -331,7 +331,10 @@ def _repo_info_from_payload(
             "Downloads": "downloads",
             "Likes": "likes",
             "CreatedAt": "created_at",
-            "UpdatedAt": "updated_at",
+            "UpdatedAt": "last_modified",
+            "LastModified": "last_modified",
+            "last_modified": "last_modified",
+            "updated_at": "last_modified",
             "Tags": "tags",
         }
         for key, value in data.items():
@@ -341,8 +344,8 @@ def _repo_info_from_payload(
         # optional ``gated`` flag) instead of the legacy ``Visibility`` integer.
         # Translate it so downstream code sees a uniform ``Visibility`` enum.
         if normalised.get("visibility") is None:
-            private_flag = normalised.pop("private", None)
-            gated_flag = normalised.pop("gated", None)
+            private_flag = normalised.get("private")
+            gated_flag = normalised.get("gated")
             if isinstance(private_flag, bool):
                 if private_flag:
                     normalised["visibility"] = Visibility.PRIVATE
@@ -845,7 +848,15 @@ def list_repos(
             page = page_number
             size = page_size
         infos = [self._repo_info_from_payload(item, rt) for item in items]
-        return PagedResult(items=infos, total_count=total, page_number=page, page_size=size)
+        # Determine collection key for OpenAPI-aligned to_dict() output
+        _COLLECTION_KEYS = {
+            RepoType.MODEL: "models",
+            RepoType.DATASET: "datasets",
+            RepoType.SKILL: "skills",
+            RepoType.MCP: "servers",
+        }
+        key = _COLLECTION_KEYS.get(rt, "items")
+        return PagedResult(items=infos, total_count=total, page_number=page, page_size=size, collection_key=key)
 
     def delete_repo(self, repo_id: str, repo_type: RepoTypeLike) -> None:
         """Delete a repository.

src/modelscope_hub/cli/repo.py

@@ -11,7 +11,7 @@
 from pathlib import Path
 
 from ..constants import RepoType
-from ..errors import is_repo_exists_error
+from ..errors import AlreadyExistsError, is_repo_exists_error
 from ..types import RepoInfo
 from .base import CLICommand, add_repo_type_arg, error, info, make_api, print_env_table, render_table, success
 from .compat import add_subcmd_token_endpoint
@@ -114,6 +114,11 @@ def execute(self) -> None:
                 **extra,
             )
             success(f"Created {self.args.repo_type}: {repo.repo_id or self.args.repo_id}")
+        except AlreadyExistsError:
+            if getattr(self.args, "exist_ok", False):
+                info(f"Repository already exists: {self.args.repo_id}")
+                return
+            raise
         except Exception as exc:
             if getattr(self.args, "exist_ok", False) and is_repo_exists_error(exc):
                 info(f"Repository already exists: {self.args.repo_id}")

src/modelscope_hub/compat/hub_api.py

@@ -17,6 +17,7 @@
 from ..api import HubApi
 from ..constants import RepoType
 from ..errors import (
+    AlreadyExistsError,
     AuthenticationError,
     InvalidParameter,
     NotExistError,
@@ -26,7 +27,7 @@
 from ..utils.logger import get_logger
 
 if TYPE_CHECKING:
-    from ..types import RepoInfo
+    from ..types import PagedResult, RepoInfo
 
 logger = get_logger("compat")
 
@@ -42,6 +43,9 @@ class LegacyHubApi:
     new HubApi implementation.
     """
 
+    _api: HubApi
+    _endpoint: str | None
+
     def __init__(
         self,
         endpoint: str | None = None,
@@ -126,6 +130,10 @@ def create_repo(
                 chinese_name=chinese_name,
                 **kwargs,
             )
+        except AlreadyExistsError:
+            if exist_ok:
+                return None
+            raise
         except Exception as exc:
             if exist_ok and is_repo_exists_error(exc):
                 return None
@@ -182,9 +190,12 @@ def push_model(self, model_id: str, model_dir: str, **kwargs: Any) -> None:
                     license=kwargs.get("license"),
                     chinese_name=kwargs.get("chinese_name"),
                 )
+            except AlreadyExistsError:
+                logger.info("Repository '%s' already exists, proceeding with upload.", model_id)
             except Exception as exc:
                 if not is_repo_exists_error(exc):
                     raise
+                logger.info("Repository '%s' already exists, proceeding with upload.", model_id)
             self._api.upload_folder(
                 model_id,
                 RepoType.MODEL,
@@ -248,6 +259,44 @@ def repo_exists(
                 raise
             return False
 
+    def list_repos(
+        self,
+        repo_type: str | RepoType,
+        *,
+        owner: str | None = None,
+        search: str | None = None,
+        sort: str | None = None,
+        page_number: int = 1,
+        page_size: int = 10,
+        **filters: Any,
+    ) -> "PagedResult[RepoInfo]":
+        """List repositories of the given type.
+
+        Delegates to :meth:`HubApi.list_repos`.
+        """
+        return self._api.list_repos(
+            repo_type,
+            owner=owner,
+            search=search,
+            sort=sort,
+            page_number=page_number,
+            page_size=page_size,
+            **filters,
+        )
+
+    def get_repo(
+        self,
+        repo_id: str,
+        repo_type: str | RepoType,
+        *,
+        revision: str | None = None,
+    ) -> "RepoInfo":
+        """Get repository information.
+
+        Delegates to :meth:`HubApi.get_repo`.
+        """
+        return self._api.get_repo(repo_id, repo_type, revision=revision)
+
     # ------------------------------------------------------------------
     # Download operations
     # ------------------------------------------------------------------
@@ -928,7 +977,8 @@ def dataset_download_statistics(
     "downloads": "Downloads",
     "likes": "Likes",
     "created_at": "CreatedAt",
-    "updated_at": "UpdatedAt",
+    "updated_at": "UpdatedAt",  # backward compat if manually constructed
+    "last_modified": "UpdatedAt",
     "license": "License",
     "tags": "Tags",
 }

src/modelscope_hub/errors.py

@@ -223,6 +223,17 @@ class InvalidParameter(APIError, ValueError):
     suggestion = "Invalid request parameters. Please check and retry."
 
 
+class AlreadyExistsError(InvalidParameter):
+    """Resource already exists (e.g., repo name is taken).
+
+    Error code: E3026
+    """
+
+    error_code = "E3026"
+    retryable = False
+    suggestion = "Resource already exists. Use exist_ok=True to ignore."
+
+
 # -- Rate limiting (E1021) --------------------------------------------------
 class RateLimitError(APIError):
     """Raised on HTTP 429 -- client should back off and retry later."""
@@ -454,6 +465,24 @@ def raise_for_status(response: "Response") -> None:
     else:
         exc_cls = _STATUS_MAP.get(status, APIError)
 
+    # Detect "already exists" errors before falling back to InvalidParameter
+    if exc_cls is InvalidParameter and isinstance(body, dict):
+        code = body.get("Code") or body.get("code")
+        msg_text = (body.get("Message") or body.get("message")
+                    or body.get("msg") or body.get("Msg") or "").lower()
+        is_exists = False
+        if code is not None:
+            try:
+                if int(code) in _ALREADY_EXISTS_CODES:
+                    is_exists = True
+            except (TypeError, ValueError):
+                pass
+        if not is_exists:
+            if any(kw in msg_text for kw in _ALREADY_EXISTS_KEYWORDS):
+                is_exists = True
+        if is_exists:
+            exc_cls = AlreadyExistsError
+
     kwargs: dict[str, Any] = dict(
         status_code=status,
         request_id=request_id,
@@ -481,13 +510,36 @@ def raise_for_status(response: "Response") -> None:
 # ---------------------------------------------------------------------------
 # Repo-exists detection (shared by cli/repo.py and compat/hub_api.py)
 # ---------------------------------------------------------------------------
-_ALREADY_EXISTS_CODES = {10020101001, 10010101001}
+_ALREADY_EXISTS_CODES: set[int] = {
+    10020101001,   # 国内站 - 数据集已存在
+    10010101001,   # 国内站 - 模型已存在
+    10010202004,   # 国际站 - 名称已被使用
+}
+
+_ALREADY_EXISTS_KEYWORDS: frozenset[str] = frozenset({
+    "exist",
+    "already",
+    "can not be used",
+    "not available",
+    "已被注册",
+    "已存在",
+    "名称不可用",
+})
 
 
 def is_repo_exists_error(exc: BaseException) -> bool:
-    """Detect "repo already exists" regardless of locale or error format."""
+    """Detect "repo already exists" errors.
+
+    With the introduction of :class:`AlreadyExistsError`, this is now
+    primarily a simple ``isinstance`` check. The keyword/code fallback
+    is retained for backward compatibility with legacy exceptions that
+    pre-date the structured error hierarchy.
+    """
+    if isinstance(exc, AlreadyExistsError):
+        return True
+    # Fallback: legacy exceptions that may not be AlreadyExistsError
     msg = str(exc).lower()
-    if "exist" in msg or "已被注册" in msg or "已存在" in msg:
+    if any(kw in msg for kw in _ALREADY_EXISTS_KEYWORDS):
         return True
     body = getattr(exc, "response_body", None)
     if isinstance(body, dict):
@@ -510,6 +562,7 @@ def is_repo_exists_error(exc: BaseException) -> bool:
     # Resource / Validation
     "NotExistError",
     "InvalidParameter",
+    "AlreadyExistsError",
     # Rate limiting / Server
     "RateLimitError",
     "ServerError",