Skip to content

Repo.clone_from does not account for kill_after_timeout #892

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
schanzel opened this issue Jul 15, 2019 · 7 comments
Open

Repo.clone_from does not account for kill_after_timeout #892

schanzel opened this issue Jul 15, 2019 · 7 comments

Comments

@schanzel
Copy link

Hi there,

as stated in the docs, there is a kill_after_timeout parameter to the GitCmd.execute method which does exactly what it's supposed to be when, e.g., given to a repo.git.push call.
But it does not seem to be considered when passed to Repo.clone_from (cf. example below).
Is this intended or a bug?
Or is there another way to freshly clone a repository and pass this argument?

repo = Repo.clone_from(some_remote, some_path, kill_after_timeout=1) # does not timeout
repo.git.push("origin", "master", kill_after_timeout=1) # this does timeout
@schanzel
Copy link
Author

I ended up with this workaround for now but I think it would be good to address this problem as clone is a remote action that could time out.

repo = Repo.init(some_path)
repo.git.remote("add", "origin", some_remote)
repo.git.pull("origin", "master", kill_after_timeout=1)

@Byron
Copy link
Member

Byron commented Jul 21, 2019

In theory, it should pass through any kwarg to clone as well. Just recently there was the multi_options flag added, which might end up doing what you want.

Even though I didn't try to reproduce the issue, I think it's worth looking at it to see what's going on.

@Yikun
Copy link

Yikun commented Mar 1, 2021

The clone_from calls git clone function with as_process=True, it skips the kill monitor and call self.AutoInterrupt(proc, command) directly.

So, if we want to change clone_from to with kill monitor, we should set the as_process = False in here, but I'm not sure the detail backgroud of the original implentation.

@Byron Would you like take a look, and give some suggestion?

@Byron
Copy link
Member

Byron commented Mar 3, 2021

Thanks for looking into it, having the links to code is very helpful.

Setting as_process=False would immediately collect all output and wait for the process to exit, making the collection of progress impossible. However, the latter is only done if there is a progress handler which isn't necessarily the case for you.

That said, it's clearly an oversight in the implementation, one might want to try adding support for killing the process after timeout for the AutoInterrupt helper as well. Note that the latter might not even work as destructors are not reliable in Python (anymore), but that's another topic.

@Yikun
Copy link

Yikun commented Mar 4, 2021

Before adding support on it, you could also use the below wordaround to support git clone with timeout:

import git
import os
mygit = git.cmd.Git(os.getcwd())
mygit.clone(
    git.cmd.Git.polish_url(git_url), local_path,
    kill_after_timeout=timeout
)

from
[1] https://opendev.org/zuul/zuul/commit/ba1c8c0e310bbe4a874aaa81042b8f1d3e8ac078

@RT4U
Copy link

RT4U commented Apr 19, 2021

Before adding support on it, you could also use the below wordaround to support git clone with timeout:

import git
import os
mygit = git.cmd.Git(os.getcwd())
mygit.clone(
    git.cmd.Git.polish_url(git_url), local_path,
    kill_after_timeout=timeout
)

from
[1] https://opendev.org/zuul/zuul/commit/ba1c8c0e310bbe4a874aaa81042b8f1d3e8ac078

How can I get the repo object from this for iter_commits?

Also specifying as_process in clone_from function results in a duplicate keyword arg exeception.

@Byron
Copy link
Member

Byron commented Apr 20, 2021

How can I get the repo object from this for iter_commits?

git.Repo(local_path) might be worth a try.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants