Failed git push does not fail release

LvffY commented 2 years ago

First of all : thanks for your tool :)

First, my issue

During a release, if a push fails, the release does not fail. This can make false positive CI/CD builds (i.e the pipeline return as green but the push did not went well, so it's in fact, a red pipeline).

Reproduce steps

Configure zest.releaser through setup.cfg file

[zest.releaser]
release = no # This one is important to see the logs
# I don't think other confs matter, but I'll leave it here, for the record
create-wheel = yes
encoding = utf-8
tag-format = {version}
extra-message = [zest.releaser] 🤖 [ci skip]
date-format = %%Y-%%m-%%d %%H:%%M:%%S

Make the release on a branch where you don't have the right to push (imagine having a branching name pattern setup in your Git manager)
run fullrelease --no-input

Expected result

The release should me marked as failed

Actual result

The release is marked as succeeded.

Here is the logs that I get :

Running fullrelease --no-input

Sans titre

In opposite, running the commands git tag test then try to push with git push origin my-branch --tags we can see that my command return a non-zero return code

Sans titre

Second, a feature request

Based on this case, it could be great to have a rollback feature, that could allow people to run some code in case of error.

I think that a default implementation could be : if something went wrong, try to remove the supposed created tag and, why not, the uploaded artifact

LvffY commented 2 years ago

@mauritsvanrees Did you have time to check on this issue ?

mauritsvanrees commented 2 years ago

@LvffY Sorry, no not yet. Let me have a look now.

In postrelease.py we call self._push.
This method is in baserelease.py.
It calls execute_command in utils.py.
When an executed command fails, the error is printed, but execution of the zest.releaser command continues.
This can only be influenced by passing allow_retry=True to execute_command. Then the user has to answer a question: do you want to retry, continue, or quit?

@reinout We do not actually use this allow_retry option anywhere, except in utils.txt where we test that this option works... I think this was mostly meant for retrying an upload, but we have the separate _retry_twine method for this now.

The easiest solution to the issue here would be to change the _push method to use allow_retry=True. Then the user will see that there is an error, and can retry, continue or quit. I thought this would fail in CI, as this will run in no-input mode, but I see we have set things up so you get a RunTimeError then, so that would work fine. What do you think? In this case though, when called from CI, the CI without input would

mauritsvanrees commented 2 years ago

@LvffY

Second, a feature request Based on this case, it could be great to have a rollback feature, that could allow people to run some code in case of error.

I think the problem here would be: how far do you roll back? Do you revert the postrelease commit? The tag? The release commit? The prerelease commit? Commits by plugins? All of these?

And if you have used this successfully to upload a new release on PyPI, then I would not want to undo any of this. I would want to keep the exact commit and tag that were used to create this release. I can then manually push the changes to a new branch and create a PR, and that will likely solve the problem.

LvffY commented 2 years ago

@mauritsvanrees on the rollback feature I think that we want to rollback everything.

You probably want to rollback the upload on the pipy repository as well.

But if the upload on pypi is not "rollbackable", may be you're right and we don't want to rollback anything ?

In this case, may be it could just be a documentation on how to do such a rollback if customer want to do so.

reinout commented 2 years ago

The easiest solution to the issue here would be to change the _push method to use allow_retry=True. Then the user will see that there is an error, and can retry, continue or quit.

Some quick thoughts:

git push is the last step, so "continue" isn't really needed. Retry/quit could be nice.
Though the necessary "git push" command should be visible for manual re-try.
Some documentation about how to roll back a partially failed release could help. I would put it in the general documentation and not print it on the commandline. (Perhaps print the url of the doc to the commandline).

The actual error that you show is caused by insufficient rights due to a protected main branch. That's actually a nasty problem. The ones making releases have to be admins that are allowed to circumvent the protection. My assumption is pretty much that the ones with pypi upload rights also have git push rights.

Anyway, such a protection isn't something that a retry is going to help with. Though... you could get someone to give you the rights and then retry it. So: the retry seems like a good idea.

reinout commented 1 year ago

I have a PR #430 that (mostly) fixes this, I hope.

zestsoftware / zest.releaser