Skip to content

Raise exception when git push fails #621

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
michaelkozakovsc opened this issue Apr 19, 2017 · 19 comments
Open

Raise exception when git push fails #621

michaelkozakovsc opened this issue Apr 19, 2017 · 19 comments

Comments

@michaelkozakovsc
Copy link

michaelkozakovsc commented Apr 19, 2017

Having to perform bit operations on flags is very unintuitive. The command line git client fails when push fails.

@NicoHood
Copy link

I have the same issue. An exception should be raised instead.

Error lines received while fetching: error: failed to push some refs to '[email protected]:NicoHood/project.git'
hint: Updates were rejected because the tag already exists in the remote.

@Byron
Copy link
Member

Byron commented Jun 10, 2017

This might be related to a change in the git program itself, as I believe git push should return with a none-zero exit status if it fails. Apparently this doesn't happen under certain circumstances (anymore).

@David-Green
Copy link

Is problem that it's eating the exception here

https://github.com/gitpython-developers/GitPython/blob/master/git/remote.py#L702

Should always raise an exception, warning text also references fetch instead of push.

@Byron
Copy link
Member

Byron commented Sep 28, 2017

Is this still an issue? The links provided don't refer to a specific commit, thus the lines shift and make it hard to find the point you refer to, in retrospective. Thank you.

@michaelkozakovsc
Copy link
Author

@Byron i think @David-Green was referring to this

elif stderr_text:

@Wombleydude
Copy link

Wombleydude commented Apr 18, 2018

This has also been an issue for me using 2.1.5:
When doing :
self.repo = Repo('repo_dir')
self.repo.remotes[0].push(self.repo.head.reference.name)
Get this logging:
(remote.py:702) WARNING > Error lines received while fetching: error: failed to push some refs to 'ssh://xxxxxxx' To prevent you from losing history, non-fast-forward updates were rejected Merge the remote changes before pushing again. See the 'Note about fast-forwards' section of 'git push --help' for details.

But in fact I want this to be an exception. It does look like the following lines that are the problem (at line 730 in the current master:
except Exception:
if not output:
raise
elif stderr_text:
log.warning("Error lines received while fetching: %s", stderr_text)
return output

@bagatonovic-remesh
Copy link

bagatonovic-remesh commented Apr 25, 2018

Sorry to pile on, but this just bit my team today too. We were moving a few hundred repos, and a few of them failed to push with mirror to the new remote, yet the script exited successfully with no errors but never actually pushed the repo. Trying to push with command line gives us "error: failed to push some refs to..."

@soumyadipdm
Copy link

soumyadipdm commented Apr 30, 2018

I got bitten by this too while trying to automate git push.

2018-04-30 11:12:37,693 WARNING git.remote Error lines received while fetching: error: failed to push some refs to 'git@xxxxxx:yyyyyy/zzzzz.git'

@Byron
Copy link
Member

Byron commented Jun 5, 2018

Do you think fixing this would be possible for one of the people having been bitten by this?
The code to alter should be right here.
Probably changing the behaviour from logging to throwing an exception would be desirable, even though it technically is a breaking change.

@JimNero009
Copy link

Also been bitten by this recently...reviving!

@impredicative
Copy link

impredicative commented Mar 4, 2019

I have had to use some very serious checking to detect and handle failed pushes with gitpython==2.1.11 and it's bonkers that I had to try so hard.

@artgoldberg
Copy link

Agree. Errors should be reported as exceptions.

@KalenWessel
Copy link

KalenWessel commented Jun 10, 2019

Also was just bit by this issue.

wmfgerrit pushed a commit to wikimedia/operations-software-netbox-extras that referenced this issue Dec 20, 2019
* GitPython doesn't raise an exception if a failure occurs when pushing
  to the remote. Handling the error based on the push flags.
  See also: gitpython-developers/GitPython#621

Bug: T233183
Change-Id: I0e52940691080baf95fa4b2da9328e4557ad395f
@rfrancois
Copy link

Also was just bit by this issue.

@Byron
Copy link
Member

Byron commented Jul 15, 2021

As a general question: What about submitting a PR with a new push_or_throw(…) method, which does as it says? There should be enough references in this thread to implement it without spending too much time on research.

Sjord added a commit to Sjord/GitPython that referenced this issue Oct 1, 2021
Sjord added a commit to Sjord/GitPython that referenced this issue Oct 1, 2021
Sjord added a commit to Sjord/GitPython that referenced this issue Oct 1, 2021
Sjord added a commit to Sjord/GitPython that referenced this issue Oct 12, 2021
List-like, so that it's backward compatible. But it has a new method
raise_on_error, that throws an exception if anything failed to push.

Related to gitpython-developers#621
Sjord added a commit to Sjord/GitPython that referenced this issue Oct 12, 2021
List-like, so that it's backward compatible. But it has a new method
raise_on_error, that throws an exception if anything failed to push.

Related to gitpython-developers#621
Sjord added a commit to Sjord/GitPython that referenced this issue Nov 8, 2021
List-like, so that it's backward compatible. But it has a new method
raise_on_error, that throws an exception if anything failed to push.

Related to gitpython-developers#621
Byron pushed a commit that referenced this issue Nov 13, 2021
List-like, so that it's backward compatible. But it has a new method
raise_on_error, that throws an exception if anything failed to push.

Related to #621
@CyrilBrulebois
Copy link

In case someone else stumbles on this very strange behaviour, and doesn't immediately dive into Sjord's commits (mentioned in this issue, above this comment): raise_if_error() can now be called on the object returned by the push() call.

That landed in 3.1.25, and the tutorial has an example:

# push and pull behaves similarly to `git push|pull`
origin.pull()
origin.push()  # attempt push, ignore errors
origin.push().raise_if_error()  # push and raise error if it fails

@CyrilBrulebois
Copy link

For some reason, the PushInfoList class has been shipped since 35f7e94 aka. 3.1.25~10 but can't be found by searching for it on https://gitpython.readthedocs.io/en/latest/ (latest or specific versions). So I suppose some extra doc metadata is needed to improve discoverability?

@CyrilBrulebois
Copy link

In case someone needs to work with a slightly older version (e.g. 3.1.14, available in Debian 11), here's something that seems to do the trick for me (keeping in mind raising a generic Exception works well enough for me since I'm in a retry loop doing fetch & push inside a try/except block):

origin.pull(rebase=True)
# Workaround for missing origin.push().raise_if_error() in 3.1.14
# (see https://github.com/gitpython-developers/GitPython/issues/621):
pushinfolist = origin.push()
for pi in pushinfolist:  # pylint: disable=invalid-name
    for flag in [pi.REJECTED, pi.REMOTE_REJECTED, pi.REMOTE_FAILURE, pi.ERROR]:
        if pi.flags & flag:
            raise Exception("error flag %d set" % flag)

Caveat: I had to hardcode which flags look like errors I should care about; not sure how stable it is going to be across releases.

@Byron
Copy link
Member

Byron commented Apr 9, 2022

Thanks for sharing! The flags should not change across releases as breaking changes are generally avoided.

wmfgerrit pushed a commit to wikimedia/operations-software-spicerack that referenced this issue Apr 27, 2022
inspired by:
 gitpython-developers/GitPython#621 (comment)

Change-Id: I98570930ac54aea57f7e632ff6599aa51d5f45d9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.