Non blocking git push #315

LysandreJik · 2021-09-06T19:54:32Z

This PR introduces a non-blocking git push as asked in #169 and proposed by @cccntu in #171.

It is currently painful to have to wait until a git push is over before continuing to something else. Since this uses nothing else than bandwidth, it is fine for it to happen behind the scenes.

This implementation:

Implements a blocking parameter to git_push and the commit context manager; these variables default to True.
In case of blocking=False, outputs a variable that can be monitored to follow the status of the git push command
In case of blocking=False, outputs a variable that can be monitored to ensure the git push command has terminated
These variables are added to the instance variable command_queue to be managed. More information about this can be found below.
This implementation leverages the atexit official lib to ensure that all processes have completed before exiting the Python runtime.
The progress bars are not managed when using a blocking=False parameter.
Implements a warning when several commits are about to be pushed. The progress bars may be unreliable as a single progress bar may monitor the upload of several versions of a single file, therefore resulting in a jump between the two uploads on the progress bars.

It is recommended to monitor the progress of the current push before attempting a second one; however, several pushes may be launched concurrently. See below for the expected behaviors:

What happens when two git pushes are done simultaneously?

If two identical git pushes are done, then no error in the workflow will happen. The remote correctly registers the updated version.

Two times more bandwidth is consumed as both pushes will upload all files.

What happens when two git pushes are done simultaneously, with the second git push having one or several additional commits?

All commits are correctly registered on the remote. More bandwidth is consumed as both pushes will upload all files.

What happens if one of the pushes errors out?

Given that the pushes are independent of each other as the remote isn't updated until a push is complete, the workflow should complete.

The `command_queue`

This implementation implements a command_queue so that users may be aware of the current status of all commands launched.

It can be accessed as follows:

# Access to command queue
command_queue = repo.command_queue

# Accessing a single command
last_command = repo.command_queue[-1]

# Vizualizing the command
print(last_command)
# [push command, status code: running, in progress. PID: 299167]

# Accessing the command's properties
last_command.status
last_command.is_done
last_command.title
last_command.stderr
last_command.stdout

# Accessing the commands that are currently in progress
commands_in_progress = repo.commands_in_progress

# Accessing the commands that failed
commands_failed = repo.commands_failed

sgugger

Very nice! Thanks a lot for working on this!

src/huggingface_hub/repository.py

osanseviero

This looks great! Thanks for the PR!

I mostly have some questions for my own learning/understanding :)

src/huggingface_hub/README.md

osanseviero · 2021-09-10T08:24:40Z

src/huggingface_hub/repository.py

@@ -344,6 +404,10 @@ def __init__(
        if revision is not None:
            self.git_checkout(revision, create_branch_ok=True)

+        # This ensures that all commands exit before exiting the Python runtime.
+        # This will ensure all pushes register on the hub, even if other errors happen in subsequent operations.


What happens if user tries to kill the process (for example ctrl + d) while things are getting pushed?

If the user tries to kill the process, it will try to continue pushing:

>>> repo.command_queue [[push command, status code: running, in progress. PID: 95581]] >>> (CTRL+C) Waiting for the following commands to finish before shutting down: [[push command, status code: running, in progress. PID: 95581]].

If you try to exit again:

^CError in atexit._run_exitfuncs: Traceback (most recent call last): File "/home/lysandre/Workspaces/Python/huggingface_hub/src/huggingface_hub/repository.py", line 1141, in wait_for_commands time.sleep(1) KeyboardInterrupt

and then you're out!

That's perfect!

osanseviero · 2021-09-10T08:27:07Z

src/huggingface_hub/repository.py

-    def clone_from(self, repo_url: str, use_auth_token: Union[bool, str, None] = None):
+    def clone_from(
+        self, repo_url: str, use_auth_token: Union[bool, str, None] = None
+    ) -> bool:


Maybe I'm missing something, but where/why are we returning a bool here?

Very nice catch, that's a mistake from my side

osanseviero · 2021-09-10T08:33:07Z

tests/test_repository.py

+        try:
+            url, result = repo.git_push(blocking=False)
+        except subprocess.CalledProcessError as exc:
+            print(exc.stderr)
+            raise exc


Why are we having a try/except here? This seems off for a test

Indeed, I suspect it was in an old test which got copy-pasted around. Fixed that in the last commit.

Add test & typings Implement status code visualization context manager has access to commands Better warning Traditional push should also contribute to queue Add push_to-hub Docs and tests Update src/huggingface_hub/README.md Co-authored-by: Omar Sanseviero <[email protected]> Apply Omar's review

sgugger approved these changes Sep 9, 2021

View reviewed changes

src/huggingface_hub/repository.py Show resolved Hide resolved

src/huggingface_hub/repository.py Show resolved Hide resolved

LysandreJik marked this pull request as ready for review September 9, 2021 19:20

LysandreJik requested a review from osanseviero September 9, 2021 19:20

sgugger mentioned this pull request Sep 9, 2021

Push to hub when saving checkpoints huggingface/transformers#13503

Merged

osanseviero approved these changes Sep 10, 2021

View reviewed changes

LysandreJik force-pushed the non-blocking-git-push branch from 6ae8928 to af19581 Compare September 13, 2021 13:34

LysandreJik added 3 commits September 13, 2021 09:47

Bigger push

0788b59

Simplify test

d294985

Bigger test

673e0ad

LysandreJik merged commit 03afa2a into main Sep 13, 2021

LysandreJik deleted the non-blocking-git-push branch September 13, 2021 15:31

This was referenced Dec 21, 2021

Non-blocking git_push #169

Closed

Add non-blocking git_push #171

Closed

This was referenced Mar 28, 2023

not able to use push_to_hub during tpu training huggingface/diffusers#2851

Closed

Log at warning level when waiting for commands to finish #1412

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Non blocking git push #315

Non blocking git push #315

LysandreJik commented Sep 6, 2021 •

edited

Loading

sgugger left a comment

osanseviero left a comment

osanseviero Sep 10, 2021

LysandreJik Sep 13, 2021

osanseviero Sep 13, 2021

osanseviero Sep 10, 2021

LysandreJik Sep 13, 2021

osanseviero Sep 10, 2021

LysandreJik Sep 13, 2021

Non blocking git push #315

Non blocking git push #315

Conversation

LysandreJik commented Sep 6, 2021 • edited Loading

What happens when two git pushes are done simultaneously?

What happens when two git pushes are done simultaneously, with the second git push having one or several additional commits?

What happens if one of the pushes errors out?

The command_queue

sgugger left a comment

Choose a reason for hiding this comment

osanseviero left a comment

Choose a reason for hiding this comment

osanseviero Sep 10, 2021

Choose a reason for hiding this comment

LysandreJik Sep 13, 2021

Choose a reason for hiding this comment

osanseviero Sep 13, 2021

Choose a reason for hiding this comment

osanseviero Sep 10, 2021

Choose a reason for hiding this comment

LysandreJik Sep 13, 2021

Choose a reason for hiding this comment

osanseviero Sep 10, 2021

Choose a reason for hiding this comment

LysandreJik Sep 13, 2021

Choose a reason for hiding this comment

LysandreJik commented Sep 6, 2021 •

edited

Loading

The `command_queue`