diff --git a/src/modelscope_hub/_upload.py b/src/modelscope_hub/_upload.py index 43d49f8..cb6cd18 100644 --- a/src/modelscope_hub/_upload.py +++ b/src/modelscope_hub/_upload.py @@ -635,6 +635,42 @@ def upload_file( revision=revision, ) + # ------------------------------------------------------------------ + # Public: delete_files + # ------------------------------------------------------------------ + def delete_files( + self, + repo_id: str, + repo_type: str, + file_paths: list[str], + *, + commit_message: str = "Delete files", + revision: str = "master", + ) -> dict: + """Delete repository files through a commit operation. + + The direct repository DELETE endpoints reject API-token authentication. + Commit ``delete`` actions use the same supported write path as uploads + and are applied atomically in a single commit. + """ + paths = list(dict.fromkeys(path for path in file_paths if path)) + if not paths: + raise InvalidParameter( + "file_paths must contain at least one non-empty path.") + + self._commit_with_retry( + repo_id=repo_id, + repo_type=repo_type, + operations=self._build_delete_operations(paths), + commit_message=commit_message, + revision=revision, + ) + return { + "deleted_files": paths, + "failed_files": [], + "total_files": len(paths), + } + # ------------------------------------------------------------------ # Public: upload_folder # ------------------------------------------------------------------ diff --git a/src/modelscope_hub/api.py b/src/modelscope_hub/api.py index 5fb1996..ac22516 100644 --- a/src/modelscope_hub/api.py +++ b/src/modelscope_hub/api.py @@ -46,7 +46,6 @@ AuthenticationError, HubError, InvalidParameter, - NetworkError, NotExistError, NotSupportedError, PermissionDeniedError, @@ -1675,12 +1674,11 @@ def delete_files( commit_message: str | None = None, revision: str | None = None, ) -> dict: - """Delete one or more files from a repository. + """Delete one or more files from a repository in a single commit. - .. note:: - File deletion is restricted by the server to cookie-based session - auth (interactive login). API tokens (``ms-...``) may receive a 401 - "token no longer supports deletion operations" error. + The direct repository DELETE endpoints reject API-token authentication. + This method therefore sends commit ``delete`` actions through the same + supported write path as :meth:`upload_file` and :meth:`upload_folder`. Parameters ---------- @@ -1691,7 +1689,7 @@ def delete_files( file_paths : iterable of str Paths of files to remove. Empty entries are ignored. commit_message : str, optional - Unused (kept for API compatibility). + Message for the delete commit. Defaults to ``"Delete files"``. revision : str, optional Branch to delete from. Defaults to ``"master"``. @@ -1699,42 +1697,16 @@ def delete_files( ------- dict Summary with ``deleted_files`` and ``failed_files`` lists. - - Raises - ------ - InvalidParameter - When ``file_paths`` resolves to an empty list. - - Examples - -------- - >>> api.delete_files( - ... "alice/llama-7b", - ... "model", - ... ["old_weights.bin", "deprecated/config.json"], - ... ) """ rt = self._normalize_repo_type(repo_type) - paths = [p for p in file_paths if p] - if not paths: - raise InvalidParameter("file_paths must contain at least one non-empty path.") - - deleted, failed = [], [] - for p in paths: - try: - self.legacy.delete_file( - repo_id=repo_id, - repo_type=str(rt), - file_path=p, - revision=revision or "master", - ) - deleted.append(p) - except (AuthenticationError, NetworkError): - failed.append(p) - raise - except Exception: - failed.append(p) - - return {"deleted_files": deleted, "failed_files": failed, "total_files": len(paths)} + paths = [file_paths] if isinstance(file_paths, str) else file_paths + return self.uploader.delete_files( + repo_id=repo_id, + repo_type=str(rt), + file_paths=[path for path in paths if path], + commit_message=commit_message or "Delete files", + revision=revision or "master", + ) # ================================================================== # Versioning diff --git a/src/modelscope_hub/version.py b/src/modelscope_hub/version.py index f710337..95f9844 100644 --- a/src/modelscope_hub/version.py +++ b/src/modelscope_hub/version.py @@ -1,3 +1,3 @@ """Version information for modelscope_hub.""" -__version__ = "0.4.0" +__version__ = "0.4.1+main" diff --git a/tests/integration/test_remote_file_ops.py b/tests/integration/test_remote_file_ops.py index 282bd5e..9b434ea 100644 --- a/tests/integration/test_remote_file_ops.py +++ b/tests/integration/test_remote_file_ops.py @@ -63,10 +63,6 @@ def test_download_file(self, tmp_path): assert local_path.exists() assert content == "hello modelscope" - @pytest.mark.xfail( - reason="Server restricts file deletion to cookie-based session auth; " - "API tokens get 401 'token no longer supports deletion operations'" - ) def test_delete_files(self): """delete_files removes the file from the repo.""" print(f"\n** repo_id: {self.repo_id}") diff --git a/tests/integration/test_sdk_api.py b/tests/integration/test_sdk_api.py index 769daef..223b9f1 100644 --- a/tests/integration/test_sdk_api.py +++ b/tests/integration/test_sdk_api.py @@ -211,7 +211,6 @@ def test_list_repo_files(self): paths = [f.path for f in files] assert "list_test.txt" in paths - @pytest.mark.xfail(reason="Server restricts file deletion to cookie-based session auth") def test_delete_files(self): self.api.upload_file( self.repo_id, diff --git a/tests/test_upload_lfs_gate.py b/tests/test_upload_lfs_gate.py index 7728cef..1b64ba2 100644 --- a/tests/test_upload_lfs_gate.py +++ b/tests/test_upload_lfs_gate.py @@ -6,9 +6,12 @@ from pathlib import Path from unittest.mock import MagicMock +import pytest + import modelscope_hub._upload as upload_module +from modelscope_hub import HubApi from modelscope_hub._upload import UploadManager -from modelscope_hub.errors import NetworkError +from modelscope_hub.errors import InvalidParameter, NetworkError def _make_manager() -> tuple[UploadManager, MagicMock]: @@ -27,6 +30,88 @@ def upload_blob(*, upload_url: str, data, size: int) -> None: return UploadManager(client, MagicMock()), client +@pytest.mark.parametrize("repo_type", ["model", "dataset"]) +def test_delete_files_commits_atomic_delete_actions(repo_type: str) -> None: + manager, client = _make_manager() + + result = manager.delete_files( + repo_id="owner/repo", + repo_type=repo_type, + file_paths=["old.bin", "nested/deprecated.json", "old.bin"], + commit_message="Remove obsolete files", + revision="main", + ) + + client.create_commit.assert_called_once_with( + repo_id="owner/repo", + repo_type=repo_type, + operations=[ + { + "action": "delete", + "path": "old.bin", + "type": "normal", + "size": 0, + "sha256": "", + "content": "", + "encoding": "", + }, + { + "action": "delete", + "path": "nested/deprecated.json", + "type": "normal", + "size": 0, + "sha256": "", + "content": "", + "encoding": "", + }, + ], + commit_message="Remove obsolete files", + revision="main", + ) + assert result == { + "deleted_files": ["old.bin", "nested/deprecated.json"], + "failed_files": [], + "total_files": 2, + } + + +def test_delete_files_rejects_empty_paths() -> None: + manager, client = _make_manager() + + with pytest.raises(InvalidParameter, match="at least one"): + manager.delete_files( + repo_id="owner/repo", repo_type="model", file_paths=["", ""]) + + client.create_commit.assert_not_called() + + +@pytest.mark.parametrize("repo_type", ["model", "dataset"]) +@pytest.mark.parametrize("file_paths", [["", "old.bin"], "old.bin"]) +def test_hub_api_delete_files_delegates_to_upload_manager( + repo_type: str, file_paths: list[str] | str +) -> None: + api = HubApi(token="test-token") + api._uploader = MagicMock() + api._uploader.delete_files.return_value = {"deleted_files": ["old.bin"]} + + result = api.delete_files( + "owner/repo", + repo_type, + file_paths, + commit_message="Remove obsolete files", + revision="main", + ) + + api._uploader.delete_files.assert_called_once_with( + repo_id="owner/repo", + repo_type=repo_type, + file_paths=["old.bin"], + commit_message="Remove obsolete files", + revision="main", + ) + assert result == {"deleted_files": ["old.bin"]} + + def test_upload_file_normal_commits_inline_without_blob_api() -> None: manager, client = _make_manager()