Repository: huggingface/huggingface_hub
Pull request #1175 (2022-11-09 13:41:14)

1162 Reorder operations correctly in commit endpoint
Fix https://github.com/huggingface/huggingface_hub/issues/1162.

**With this PR:**
- Order of the operations in the commit payload is the same as the input of `create_commit(...)`. It was not the case before which causes trouble if someone wants to delete then create a file.
- If 2 operations are modifying the same file, a warning is issued. Only case without warning is "delete a file then add this file" or "delete folder then add file that would have been in this folder".
- `CommitOperationAdd` is validated at initialization (no need for `.validate()` or `._upload_info()` calls anymore)

**Warning:** I made some breaking changes in `validate_preupload_info`, `fetch_upload_modes` and `prepare_commit_payload`. I did not care too much as those 3 methods are defined in a private module and used exclusively for the in `create_commit` . I checked for safety and it's not used anywhere in `diffusers`, `transformers`, `skops` and `datasets`.

**Potential issue:** if someone adds twice the same file, once a LFS content, once a regular content then the final upload mode will be the one from the second operation. Might cause issue with request payload or gitaly. Anyway, this is a weird case so let's not care of the user has an error. A warning message is triggered before upload so the user should notice the problem. (note: this issue was already existing before this PR)


(cc @severo who raised the issue)

HuggingFaceDocBuilder: The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/huggingface_hub/pr_1175). All of your documentation changes will be reflected on that endpoint.
HuggingFaceDocBuilder: The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/huggingface_hub/pr_1175). All of your documentation changes will be reflected on that endpoint.
HuggingFaceDocBuilder: The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/huggingface_hub/pr_1175). All of your documentation changes will be reflected on that endpoint.
HuggingFaceDocBuilder: The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/huggingface_hub/pr_1175). All of your documentation changes will be reflected on that endpoint.
HuggingFaceDocBuilder: The docs for this PR live [here](https://moon-ci-docs.huggingface.co/docs/huggingface_hub/pr_1175). All of your documentation changes will be reflected on that endpoint.
Wauplin: Thanks for the reviews @coyotte508 @LysandreJik ! I'm merging :) 

Patch:
diff --git a/src/huggingface_hub/_commit_api.py b/src/huggingface_hub/_commit_api.py
index 1b9a3c20..b7b0d754 100644
--- a/src/huggingface_hub/_commit_api.py
+++ b/src/huggingface_hub/_commit_api.py
@@ -4,11 +4,14 @@ Type definitions and utilities for the `create_commit` API
 import base64
 import io
 import os
+import warnings
+from collections import defaultdict
 from concurrent import futures
 from concurrent.futures import ThreadPoolExecutor
 from contextlib import contextmanager
 from dataclasses import dataclass, field
-from typing import Any, BinaryIO, Dict, Iterable, List, Optional, Tuple, Union
+from pathlib import PurePosixPath
+from typing import Any, BinaryIO, Dict, Iterable, List, Optional, Union
 
 import requests
 
@@ -21,6 +24,7 @@ from .utils import (
     logging,
     validate_hf_hub_args,
 )
+from .utils._deprecation import _deprecate_method
 from .utils._typing import Literal
 
 
@@ -29,8 +33,6 @@ logger = logging.get_logger(__name__)
 
 UploadMode = Literal["lfs", "regular"]
 
-CommitOperationT = Union["CommitOperationAdd", "CommitOperationDelete"]
-
 
 @dataclass
 class CommitOperationDelete:
@@ -65,35 +67,35 @@ class CommitOperationDelete:
 @dataclass
 class CommitOperationAdd:
     """
-    Data structure holding necessary info to upload a file
-    to a repository on the Hub
+    Data structure holding necessary info to upload a file to a repository on the Hub.
 
     Args:
         path_in_repo (`str`):
-            Relative filepath in the repo, for example:
-            `"checkpoints/1fec34a/weights.bin"`
+            Relative filepath in the repo, for example: `"checkpoints/1fec34a/weights.bin"`
         path_or_fileobj (`str`, `bytes`, or `BinaryIO`):
             Either:
             - a path to a local file (as str) to upload
             - a buffer of bytes (`bytes`) holding the content of the file to upload
             - a "file object" (subclass of `io.BufferedIOBase`), typically obtained
                 with `open(path, "rb")`. It must support `seek()` and `tell()` methods.
+
+    Raises:
+        [`ValueError`](https://docs.python.org/3/library/exceptions.html#ValueError)
+            If `path_or_fileobj` is not one of `str`, `bytes` or `io.BufferedIOBase`.
+        [`ValueError`](https://docs.python.org/3/library/exceptions.html#ValueError)
+            If `path_or_fileobj` is a `str` but not a path to an existing file.
+        [`ValueError`](https://docs.python.org/3/library/exceptions.html#ValueError)
+            If `path_or_fileobj` is a `io.BufferedIOBase` but it doesn't support both
+            `seek()` and `tell()`.
     """
 
     path_in_repo: str
     path_or_fileobj: Union[str, bytes, BinaryIO]
+    upload_info: UploadInfo = field(init=False, repr=False)
 
-    __upload_info: Optional[UploadInfo] = field(default=None, init=False)
-
-    def validate(self):
-        """
-        Ensures `path_or_fileobj` is valid:
-        - Ensures it is either a `str`, `bytes` or an instance of `io.BufferedIOBase`
-        - If it is a `str`, ensure that it is a file path and that the file exists
-        - If it is an instance of `io.BufferedIOBase`, ensures it supports `seek()` and `tell()`
-
-        Raises: `ValueError` if `path_or_fileobj` is not valid
-        """
+    def __post_init__(self) -> None:
+        """Validates `path_or_fileobj` and compute `upload_info`."""
+        # Validate `path_or_fileobj` value
         if isinstance(self.path_or_fileobj, str):
             path_or_fileobj = os.path.normpath(os.path.expanduser(self.path_or_fileobj))
             if not os.path.isfile(path_or_fileobj):
@@ -118,22 +120,23 @@ class CommitOperationAdd:
                     " seek() and tell()"
                 ) from exc
 
-    def _upload_info(self) -> UploadInfo:
-        """
-        Computes and caches UploadInfo for the underlying data behind `path_or_fileobj`
-        Triggers `self.validate`.
+        # Compute "upload_info" attribute
+        if isinstance(self.path_or_fileobj, str):
+            self.upload_info = UploadInfo.from_path(self.path_or_fileobj)
+        elif isinstance(self.path_or_fileobj, bytes):
+            self.upload_info = UploadInfo.from_bytes(self.path_or_fileobj)
+        else:
+            self.upload_info = UploadInfo.from_fileobj(self.path_or_fileobj)
 
-        Raises: `ValueError` if self.validate fails
-        """
-        self.validate()
-        if self.__upload_info is None:
-            if isinstance(self.path_or_fileobj, str):
-                self.__upload_info = UploadInfo.from_path(self.path_or_fileobj)
-            elif isinstance(self.path_or_fileobj, bytes):
-                self.__upload_info = UploadInfo.from_bytes(self.path_or_fileobj)
-            else:
-                self.__upload_info = UploadInfo.from_fileobj(self.path_or_fileobj)
-        return self.__upload_info
+    @_deprecate_method(
+        version="0.14", message="Operation is validated at initialization."
+    )
+    def validate(self) -> None:
+        pass
+
+    @_deprecate_method(version="0.14", message="Use `upload_info` property instead.")
+    def _upload_info(self) -> UploadInfo:
+        return self.upload_info
 
     @contextmanager
     def as_file(self):
@@ -141,10 +144,6 @@ class CommitOperationAdd:
         A context manager that yields a file-like object allowing to read the underlying
         data behind `path_or_fileobj`.
 
-        Triggers `self.validate`.
-
-        Raises: `ValueError` if self.validate fails
-
         Example:
 
         ```python
@@ -152,14 +151,13 @@ class CommitOperationAdd:
         ...        path_in_repo="remote/dir/weights.h5",
         ...        path_or_fileobj="./local/weights.h5",
         ... )
-        CommitOperationAdd(path_in_repo='remote/dir/weights.h5', path_or_fileobj='./local/weights.h5', _upload_info=None)
+        CommitOperationAdd(path_in_repo='remote/dir/weights.h5', path_or_fileobj='./local/weights.h5')
 
         >>> with operation.as_file() as file:
         ...     content = file.read()
         ```
 
         """
-        self.validate()
         if isinstance(self.path_or_fileobj, str):
             with open(self.path_or_fileobj, "rb") as file:
                 yield file
@@ -183,10 +181,56 @@ class CommitOperationAdd:
 CommitOperation = Union[CommitOperationAdd, CommitOperationDelete]
 
 
+def warn_on_overwriting_operations(operations: List[CommitOperation]) -> None:
+    """
+    Warn user when a list of operations is expected to overwrite itself in a single
+    commit.
+
+    Rules:
+    - If a filepath is updated by multiple `CommitOperationAdd` operations, a warning
+      message is triggered.
+    - If a filepath is updated at least once by a `CommitOperationAdd` and then deleted
+      by a `CommitOperationDelete`, a warning is triggered.
+    - If a `CommitOperationDelete` deletes a filepath that is then updated by a
+      `CommitOperationAdd`, no warning is triggered. This is usually useless (no need to
+      delete before upload) but can happen if a user deletes an entire folder and then
+      add new files to it.
+    """
+    nb_additions_per_path: Dict[str, int] = defaultdict(int)
+    for operation in operations:
+        path_in_repo = operation.path_in_repo
+        if isinstance(operation, CommitOperationAdd):
+            if nb_additions_per_path[path_in_repo] > 0:
+                warnings.warn(
+                    "About to update multiple times the same file in the same commit:"
+                    f" '{path_in_repo}'. This can cause undesired inconsistencies in"
+                    " your repo."
+                )
+            nb_additions_per_path[path_in_repo] += 1
+            for parent in PurePosixPath(path_in_repo).parents:
+                # Also keep track of number of updated files per folder
+                # => warns if deleting a folder overwrite some contained files
+                nb_additions_per_path[str(parent)] += 1
+        if isinstance(operation, CommitOperationDelete):
+            if nb_additions_per_path[str(PurePosixPath(path_in_repo))] > 0:
+                if operation.is_folder:
+                    warnings.warn(
+                        "About to delete a folder containing files that have just been"
+                        f" updated within the same commit: '{path_in_repo}'. This can"
+                        " cause undesired inconsistencies in your repo."
+                    )
+                else:
+                    warnings.warn(
+                        "About to delete a file that have just been updated within the"
+                        f" same commit: '{path_in_repo}'. This can cause undesired"
+                        " inconsistencies in your repo."
+                    )
+
+
 @validate_hf_hub_args
 def upload_lfs_files(
     *,
-    additions: Iterable[CommitOperationAdd],
+    additions: List[CommitOperationAdd],
     repo_type: str,
     repo_id: str,
     token: Optional[str],
@@ -200,7 +244,7 @@ def upload_lfs_files(
         - LFS Batch API: https://github.com/git-lfs/git-lfs/blob/main/docs/api/batch.md
 
     Args:
-        additions (`Iterable` of `CommitOperationAdd`):
+        additions (`List` of `CommitOperationAdd`):
             The files to be uploaded
         repo_type (`str`):
             Type of the repo to upload to: `"model"`, `"dataset"` or `"space"`.
@@ -227,7 +271,7 @@ def upload_lfs_files(
     batch_actions: List[Dict] = []
     for chunk in chunk_iterable(additions, chunk_size=256):
         batch_actions_chunk, batch_errors_chunk = post_lfs_batch_info(
-            upload_infos=[op._upload_info() for op in chunk],
+            upload_infos=[op.upload_info for op in chunk],
             token=token,
             repo_id=repo_id,
             repo_type=repo_type,
@@ -248,7 +292,7 @@ def upload_lfs_files(
         batch_actions += batch_actions_chunk
 
     # Step 2: upload files concurrently according to these instructions
-    oid2addop = {add_op._upload_info().sha256.hex(): add_op for add_op in additions}
+    oid2addop = {add_op.upload_info.sha256.hex(): add_op for add_op in additions}
     with ThreadPoolExecutor(max_workers=num_threads) as pool:
         logger.debug(
             f"Uploading {len(batch_actions)} LFS files to the Hub using up to"
@@ -298,7 +342,7 @@ def _upload_lfs_object(
     large file storage.
 
     Args:
-        operation (`CommitOprationAdd`):
+        operation (`CommitOperationAdd`):
             The add operation triggering this upload
         lfs_batch_action (`dict`):
             Upload instructions from the LFS batch endpoint for this object.
@@ -309,7 +353,7 @@ def _upload_lfs_object(
     Raises: `ValueError` if `lfs_batch_action` is improperly formatted
     """
     _validate_batch_actions(lfs_batch_action)
-    upload_info = operation._upload_info()
+    upload_info = operation.upload_info
     actions = lfs_batch_action.get("actions")
     if actions is None:
         # The file was already uploaded
@@ -333,7 +377,7 @@ def _upload_lfs_object(
         logger.debug(f"{operation.path_in_repo}: Upload successful")
 
 
-def validate_preupload_info(preupload_info: dict):
+def _validate_preupload_info(preupload_info: dict):
     files = preupload_info.get("files")
     if not isinstance(files, list):
         raise ValueError("preupload_info is improperly formatted")
@@ -356,7 +400,7 @@ def fetch_upload_modes(
     token: Optional[str],
     revision: str,
     endpoint: Optional[str] = None,
-) -> List[Tuple[CommitOperationAdd, UploadMode]]:
+) -> Dict[str, UploadMode]:
     """
     Requests the Hub "preupload" endpoint to determine wether each input file
     should be uploaded as a regular git blob or as git LFS blob.
@@ -375,31 +419,28 @@ def fetch_upload_modes(
         revision (`str`):
             The git revision to upload the files to. Can be any valid git revision.
 
-    Returns:
-        list of 2-tuples, the first element being the add operation and
-        the second element the associated upload mode
+    Returns: `Dict[str, UploadMode]`
+        Key is the file path, value is the upload mode ("regular" or "lfs").
 
     Raises:
-        :class:`requests.HTTPError`:
-            If the Hub API returned an error
-        :class:`ValueError`:
-            If the Hub API returned an HTTP 400 error (bad request)
+        [`~utils.HfHubHTTPError`]
+            If the Hub API returned an error.
+        [`ValueError`](https://docs.python.org/3/library/exceptions.html#ValueError)
+            If the Hub API response is improperly formatted.
     """
     endpoint = endpoint if endpoint is not None else ENDPOINT
     headers = build_hf_headers(token=token)
 
     # Fetch upload mode (LFS or regular) chunk by chunk.
-    path2mode: Dict[str, UploadMode] = {}
+    upload_modes: Dict[str, UploadMode] = {}
     for chunk in chunk_iterable(additions, 256):
         payload = {
             "files": [
                 {
                     "path": op.path_in_repo,
-                    "sample": base64.b64encode(op._upload_info().sample).decode(
-                        "ascii"
-                    ),
-                    "size": op._upload_info().size,
-                    "sha": op._upload_info().sha256.hex(),
+                    "sample": base64.b64encode(op.upload_info.sample).decode("ascii"),
+                    "size": op.upload_info.size,
+                    "sha": op.upload_info.sha256.hex(),
                 }
                 for op in chunk
             ]
@@ -411,17 +452,17 @@ def fetch_upload_modes(
             headers=headers,
         )
         hf_raise_for_status(resp)
-        preupload_info = validate_preupload_info(resp.json())
-        path2mode.update(
+        preupload_info = _validate_preupload_info(resp.json())
+        upload_modes.update(
             **{file["path"]: file["uploadMode"] for file in preupload_info["files"]}
         )
 
-    return [(op, path2mode[op.path_in_repo]) for op in additions]
+    return upload_modes
 
 
 def prepare_commit_payload(
-    additions: Iterable[Tuple[CommitOperationAdd, UploadMode]],
-    deletions: Iterable[CommitOperationDelete],
+    operations: Iterable[CommitOperation],
+    upload_modes: Dict[str, UploadMode],
     commit_message: str,
     commit_description: Optional[str] = None,
     parent_commit: Optional[str] = None,
@@ -444,40 +485,44 @@ def prepare_commit_payload(
         header_value["parentCommit"] = parent_commit
     yield {"key": "header", "value": header_value}
 
-    # 2. Send regular files, one per line
-    yield from (
-        {
-            "key": "file",
-            "value": {
-                "content": add_op.b64content().decode(),
-                "path": add_op.path_in_repo,
-                "encoding": "base64",
-            },
-        }
-        for (add_op, upload_mode) in additions
-        if upload_mode == "regular"
-    )
-
-    # 3. Send LFS files, one per line
-    yield from (
-        {
-            "key": "lfsFile",
-            "value": {
-                "path": add_op.path_in_repo,
-                "algo": "sha256",
-                "oid": add_op._upload_info().sha256.hex(),
-                "size": add_op._upload_info().size,
-            },
-        }
-        for (add_op, upload_mode) in additions
-        if upload_mode == "lfs"
-    )
-
-    # 4. Send deleted files, one per line
-    yield from (
-        {
-            "key": "deletedFolder" if del_op.is_folder else "deletedFile",
-            "value": {"path": del_op.path_in_repo},
-        }
-        for del_op in deletions
-    )
+    # 2. Send operations, one per line
+    for operation in operations:
+        # 2.a. Case adding a regular file
+        if (
+            isinstance(operation, CommitOperationAdd)
+            and upload_modes.get(operation.path_in_repo) == "regular"
+        ):
+            yield {
+                "key": "file",
+                "value": {
+                    "content": operation.b64content().decode(),
+                    "path": operation.path_in_repo,
+                    "encoding": "base64",
+                },
+            }
+        # 2.b. Case adding an LFS file
+        elif (
+            isinstance(operation, CommitOperationAdd)
+            and upload_modes.get(operation.path_in_repo) == "lfs"
+        ):
+            yield {
+                "key": "lfsFile",
+                "value": {
+                    "path": operation.path_in_repo,
+                    "algo": "sha256",
+                    "oid": operation.upload_info.sha256.hex(),
+                    "size": operation.upload_info.size,
+                },
+            }
+        # 2.c. Case deleting a file or folder
+        elif isinstance(operation, CommitOperationDelete):
+            yield {
+                "key": "deletedFolder" if operation.is_folder else "deletedFile",
+                "value": {"path": operation.path_in_repo},
+            }
+        # 2.d. Never expected to happen
+        else:
+            raise ValueError(
+                f"Unknown operation to commit. Operation: {operation}. Upload mode:"
+                f" {upload_modes.get(operation.path_in_repo)}"
+            )
diff --git a/src/huggingface_hub/hf_api.py b/src/huggingface_hub/hf_api.py
index 15651aa9..f3541968 100644
--- a/src/huggingface_hub/hf_api.py
+++ b/src/huggingface_hub/hf_api.py
@@ -32,6 +32,7 @@ from ._commit_api import (
     fetch_upload_modes,
     prepare_commit_payload,
     upload_lfs_files,
+    warn_on_overwriting_operations,
 )
 from .community import (
     Discussion,
@@ -1867,24 +1868,19 @@ class HfApi:
 
         operations = list(operations)
         additions = [op for op in operations if isinstance(op, CommitOperationAdd)]
-        deletions = [op for op in operations if isinstance(op, CommitOperationDelete)]
-
-        if len(additions) + len(deletions) != len(operations):
-            raise ValueError(
-                "Unknown operation, must be one of `CommitOperationAdd` or"
-                " `CommitOperationDelete`"
-            )
+        nb_additions = len(additions)
+        nb_deletions = len(operations) - nb_additions
 
         logger.debug(
             f"About to commit to the hub: {len(additions)} addition(s) and"
-            f" {len(deletions)} deletion(s)."
+            f" {nb_deletions} deletion(s)."
         )
 
-        for addition in additions:
-            addition.validate()
+        # If updating twice the same file or update then delete a file in a single commit
+        warn_on_overwriting_operations(operations)
 
         try:
-            additions_with_upload_mode = fetch_upload_modes(
+            upload_modes = fetch_upload_modes(
                 additions=additions,
                 repo_type=repo_type,
                 repo_id=repo_id,
@@ -1899,8 +1895,8 @@ class HfApi:
         upload_lfs_files(
             additions=[
                 addition
-                for (addition, upload_mode) in additions_with_upload_mode
-                if upload_mode == "lfs"
+                for addition in additions
+                if upload_modes[addition.path_in_repo] == "lfs"
             ],
             repo_type=repo_type,
             repo_id=repo_id,
@@ -1909,8 +1905,8 @@ class HfApi:
             num_threads=num_threads,
         )
         commit_payload = prepare_commit_payload(
-            additions=additions_with_upload_mode,
-            deletions=deletions,
+            operations=operations,
+            upload_modes=upload_modes,
             commit_message=commit_message,
             commit_description=commit_description,
             parent_commit=parent_commit,
@@ -1940,7 +1936,7 @@ class HfApi:
             e.append_to_message(_CREATE_COMMIT_NO_REPO_ERROR_MESSAGE)
             raise
         except EntryNotFoundError as e:
-            if len(deletions) > 0 and "A file with this name doesn't exist" in str(e):
+            if nb_deletions > 0 and "A file with this name doesn't exist" in str(e):
                 e.append_to_message(
                     "\nMake sure to differentiate file and folder paths in delete"
                     " operations with a trailing '/' or using `is_folder=True/False`."


Test patch:
diff --git a/tests/test_commit_api.py b/tests/test_commit_api.py
index 1da4e2ca..facd27c6 100644
--- a/tests/test_commit_api.py
+++ b/tests/test_commit_api.py
@@ -1,6 +1,10 @@
 import unittest
 
-from huggingface_hub._commit_api import CommitOperationDelete
+from huggingface_hub._commit_api import (
+    CommitOperationAdd,
+    CommitOperationDelete,
+    warn_on_overwriting_operations,
+)
 
 
 class TestCommitOperationDelete(unittest.TestCase):
@@ -46,3 +50,49 @@ class TestCommitOperationDelete(unittest.TestCase):
     def test_is_folder_wrong_value(self):
         with self.assertRaises(ValueError):
             CommitOperationDelete(path_in_repo="path/to/folder", is_folder="any value")
+
+
+class TestWarnOnOverwritingOperations(unittest.TestCase):
+
+    add_file_ab = CommitOperationAdd(path_in_repo="a/b.txt", path_or_fileobj=b"data")
+    add_file_abc = CommitOperationAdd(path_in_repo="a/b/c.md", path_or_fileobj=b"data")
+    add_file_abd = CommitOperationAdd(path_in_repo="a/b/d.md", path_or_fileobj=b"data")
+    update_file_abc = CommitOperationAdd(
+        path_in_repo="a/b/c.md", path_or_fileobj=b"updated data"
+    )
+    delete_file_abc = CommitOperationDelete(path_in_repo="a/b/c.md")
+    delete_folder_a = CommitOperationDelete(path_in_repo="a/")
+    delete_folder_e = CommitOperationDelete(path_in_repo="e/")
+
+    def test_no_overwrite(self) -> None:
+        warn_on_overwriting_operations(
+            [
+                self.add_file_ab,
+                self.add_file_abc,
+                self.add_file_abd,
+                self.delete_folder_e,
+            ]
+        )
+
+    def test_add_then_update_file(self) -> None:
+        with self.assertWarns(UserWarning):
+            warn_on_overwriting_operations([self.add_file_abc, self.update_file_abc])
+
+    def test_add_then_delete_file(self) -> None:
+        with self.assertWarns(UserWarning):
+            warn_on_overwriting_operations([self.add_file_abc, self.delete_file_abc])
+
+    def test_add_then_delete_folder(self) -> None:
+        with self.assertWarns(UserWarning):
+            warn_on_overwriting_operations([self.add_file_abc, self.delete_folder_a])
+
+        with self.assertWarns(UserWarning):
+            warn_on_overwriting_operations([self.add_file_ab, self.delete_folder_a])
+
+    def test_delete_file_then_add(self) -> None:
+        warn_on_overwriting_operations([self.delete_file_abc, self.add_file_abc])
+
+    def test_delete_folder_then_add(self) -> None:
+        warn_on_overwriting_operations(
+            [self.delete_folder_a, self.add_file_ab, self.add_file_abc]
+        )
diff --git a/tests/test_hf_api.py b/tests/test_hf_api.py
index 91df91d0..0ee0b6ca 100644
--- a/tests/test_hf_api.py
+++ b/tests/test_hf_api.py
@@ -380,24 +380,21 @@ class CommitApiTest(HfApiCommonTestWithLogin):
 
     def test_commit_operation_validation(self):
         with open(self.tmp_file, "rt") as ftext:
-            operation = CommitOperationAdd(
-                path_or_fileobj=ftext,  # type: ignore
-                path_in_repo="README.md",
-            )
             with self.assertRaises(
                 ValueError,
                 msg="If you passed a file-like object, make sure it is in binary mode",
             ):
-                operation.validate()
+                CommitOperationAdd(
+                    path_or_fileobj=ftext, path_in_repo="README.md"  # type: ignore
+                )
 
-        operation = CommitOperationAdd(
-            path_or_fileobj=os.path.join(self.tmp_dir, "nofile.pth"),
-            path_in_repo="README.md",
-        )
         with self.assertRaises(
             ValueError, msg="path_or_fileobj is str but does not point to a file"
         ):
-            operation.validate()
+            CommitOperationAdd(
+                path_or_fileobj=os.path.join(self.tmp_dir, "nofile.pth"),
+                path_in_repo="README.md",
+            )
 
     @retry_endpoint
     def test_upload_file_path(self):
@@ -1064,7 +1061,7 @@ class CommitApiTest(HfApiCommonTestWithLogin):
                 endpoint=ENDPOINT_STAGING,
             )
             self.assertEqual(len(res), 1300)
-            for _, mode in res:
+            for _, mode in res.items():
                 self.assertEqual(mode, "lfs")
         except Exception as err:
             self.fail(err)
diff --git a/tests/test_keras_integration.py b/tests/test_keras_integration.py
index 378a60e1..5cb8f56e 100644
--- a/tests/test_keras_integration.py
+++ b/tests/test_keras_integration.py
@@ -526,9 +526,6 @@ class HubKerasSequentialTest(CommonKerasTest):
                 local_dir=tmpdirname, clone_from=ENDPOINT_STAGING + "/" + repo_id
             )
             from_pretrained_keras(tmpdirname)
-            self.assertRaises(
-                ValueError, msg="Exception encountered when calling layer*"
-            )
 
         self._api.delete_repo(repo_id=f"{REPO_NAME}")