Prevent empty commits if files did not change #2389
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
| sha.update(b"blob ") | ||
| sha.update(str(len(data)).encode()) | ||
| sha.update(b"\0") | ||
| sha.update(data) |
Co-authored-by: Julien Chaumond <julien@huggingface.co>
| @@ -246,6 +251,29 @@ def b64content(self) -> bytes: | |||
| with self.as_file() as file: | |||
| return base64.b64encode(file.read()) | |||
|
|
|||
| @property | |||
| def _local_oid(self) -> Optional[str]: | |||
There was a problem hiding this comment.
out of curiosity, how long does this take to compute on larger files?
There was a problem hiding this comment.
Asking as large file uploads (without going through your fantastic large-file-upload method) are already taking quite a bit of time before uploading anything
There was a problem hiding this comment.
For LFS files (so any file larger than 10MB for text files or 1MB for binary files) it won't add any time as we already compute and have the sha256.
For non-LFS files i expect this to be quite fast unless there are many many small files
There was a problem hiding this comment.
As @julien-c said, it should be neglectable on regular files since they are small. In any case, all regular files are loaded in memory in the /commit payload and cannot be above 1GB so having 1000s of files is not a use case to optimize for.
|
Thanks everyone for having a look at it! |
Following server-side update in https://github.com/huggingface-internal/moon-landing/pull/10547 (internal), we can now retrieve the
oidof the existing files in the repo. This allows us to determine which files did not change since the last commit and therefore avoid empty commits.This PR:
oidof the remote files in the/preuploadcall. If the file doesn't exist, nothing is returned.oidCommitOperationAddfrom the commit + log something (INFO level)Nonemight break downstream libraries, we return theCommitInfoof the last commit for that revisionNotes:
sha256in the/preuploadcall is not needed in the end so I removed it. It's needed only for the LFS uploadsforce_commit: boolparameter but honestly let's stay without it as long as it's not requested by usersThis PR closes #1411 (cc @HansBug @narugo1992).
Techincally the
oidis not exactlygit-hash-object(the "git-sha1") defined by git. For LFS files, the git hash object is supposed to be the git-sha1 of the pointer file, not the actual file. But in our use case and for simplicity, the server returns:git-hash-object(git-sha1) of the file for regular filessha256of the file for LFS filesThis is not 100% compliant but it's convenient since sha256 is already computed client-side.