Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a timeout for git commands #692

Merged
merged 2 commits into from
Feb 22, 2024
Merged

Add a timeout for git commands #692

merged 2 commits into from
Feb 22, 2024

Conversation

jcouball
Copy link
Member

@jcouball jcouball commented Feb 2, 2024

Your checklist for this pull request

馃毃Please review the guidelines for contributing to this repository.

  • Ensure all commits include DCO sign-off.
  • Ensure that your contributions pass unit testing.
  • Ensure that your contributions contain documentation if applicable.

Description

This PR adds the ability to specify a timeout (either globally or for a specific git command). Addresses feature request in #690.

From the updated README.md:

A timeout for git operations can be set either globally or for specific method calls
that accept a :timeout parameter.

The timeout value must be a real, non-negative Numeric value that specifies a
number of seconds a git command will be given to complete before being sent a KILL
signal. This library may hang if the git command does not terminate after receiving
the KILL signal.

When a command times out, a Git::SignaledError is raised.

If the timeout value is 0 or nil, no timeout will be enforced.

If a method accepts a :timeout parameter and a receives a non-nil value, it will
override the global timeout value. In this context, a value of nil (which is
usually the default) will use the global timeout value and a value of 0 will turn
off timeout enforcement for that method call no matter what the global value is.

To set a global timeout, use the Git.config object:

Git.config.timeout = nil # a value of nil or 0 means no timeout is enforced
Git.config.timeout = 1.5 # can be any real, non-negative Numeric interpreted as number of seconds

The global timeout can be overridden for a specific method if the method accepts a
:timeout parameter:

repo_url = 'https://github.com/ruby-git/ruby-git.git'
Git.clone(repo_url) # Use the global timeout value
Git.clone(repo_url, timeout: nil) # Also uses the global timeout value
Git.clone(repo_url, timeout: 0) # Do not enforce a timeout
Git.clone(repo_url, timeout: 10.5)  # Timeout after 10.5 seconds raising Git::SignaledError

If the command takes too long, a Git::SignaledError will be raised:

begin
  Git.clone(repo_url, timeout: 10)
rescue Git::TimeoutError => e
  result = e.result
  result.class #=> Git::CommandLineResult
  result.status #=> #<Process::Status: pid 62173 SIGKILL (signal 9)>
  result.status.timeout? #=> true
  result.git_cmd # The git command ran as an array of strings
  result.stdout # The command's output to stdout until it was terminated
  result.stderr # The command's output to stderr until it was terminated
end

@nckbowen
Copy link

nckbowen commented Feb 2, 2024

Looks good to me. Apart from being able to specify the timeout on an individual operation I'd want to be able to differentiate a timeout error vs. other errors which looks possible with the Git::SignaledError. Thanks for taking a look at this!

@jcouball
Copy link
Member Author

jcouball commented Feb 3, 2024

I'd want to be able to differentiate a timeout error vs. other errors which looks possible with the Git::SignaledError.

Drat! I was thinking the same but couldn't find an easy way to do that. I'll dig deeper and make it happen.

@jcouball
Copy link
Member Author

jcouball commented Feb 3, 2024

@nckbowen thank you for the feedback!

I updated the description above to indicate that if a timeout occurs, a Git::TimeoutError will be raised instead of a Git::SignaledError.

Additionally, there will be a timeout? attribute on the e.result.status object that you can inspect. This way you can either rescue Git::TimeoutError to specifically deal with a timeout or rescue Git::ExecutionError to deal with all possible execution errors: Git::TimeoutError, Git::SignaledError, or Git::FailedError and have logic that inspects the status timeout? attribute.

I would appreciate a thumbs up you are good with this design or additional feedback.

Signed-off-by: James Couball <jcouball@yahoo.com>
@jcouball jcouball force-pushed the timeout branch 2 times, most recently from 8581d85 to 025c5f8 Compare February 5, 2024 17:05
Signed-off-by: James Couball <jcouball@yahoo.com>
@jcouball jcouball merged commit 023017b into master Feb 22, 2024
8 of 10 checks passed
@jcouball jcouball deleted the timeout branch February 22, 2024 02:25
@jcouball jcouball mentioned this pull request Feb 24, 2024
jcouball added a commit that referenced this pull request Mar 19, 2024
* Implement the new timeout feature
Signed-off-by: James Couball <jcouball@yahoo.com>
jcouball added a commit that referenced this pull request Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants