[SPARK-31860][BUILD] only push release tags on succes

[holdenk]

What changes were proposed in this pull request?

Only push the release tag after the build has finished.

Why are the changes needed?

If the build fails we don't need a release tag.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Running locally with a fake user in #28667

[holdenk]

cc @dongjoon-hyun here's the master PR

[dongjoon-hyun]

Thank you, @holdenk .

[dongjoon-hyun]

cc @dbtsai

[dbtsai]

LGTM.

[SparkQA]

Test build #123402 has finished for PR 28700 at commit b38181d.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/28063/
Test FAILed.

[cloud-fan]

with this patch, does it mean we have to re-do all the release steps if one step failed?

[holdenk]

Yes, although that seems like the right thing to do anyways.

On Fri, Jun 5, 2020 at 7:22 PM Wenchen Fan ***@***.***> wrote: with this patch, does it mean we have to re-do all the release steps if one step failed? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#28700 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAOT5ODK5MV4INUP3ARVZLRVGR4RANCNFSM4NQHA4CQ> .

-- Cell : 425-233-8271

[cloud-fan]

We only need to re-do all the release steps if the commit of the RC is problematic (e.g. fails the tests). Otherwise, it's OK to push the RC git tag first. When running the release script, it's possible that one step fails with some transient error like a network issue, and it's really a waste of time to re-run all the steps. The script provides the -s option to only run a single step, which can save a lot of time if only one step fails.

I had to revert this commit when I was helping @rxin to cut the RC, as I hit transient errors quite a bit. I think it's very uncommon that we cut the RC tag with a wrong commit, but if it happens, I think it's OK to start the next RC immediately.

Shall we revert it?

[dongjoon-hyun]

@cloud-fan . Yes, it does. BTW, this was merged to branch-2.4 first when @holdenk did 2.4.6 and you also agreed like the following at that time. And, this PR is a forward port after merging to branch-2.4 to unblock her. (also cc @srowen )

• [SPARK-31860][BUILD][2.4] only push release tags on success #28667 (comment)

If this patch doesn't fit branch-3.0 due to branch-3.0's flakiness, I'm +1 for @cloud-fan 's request. These release scripts exist to help release managers. 3.0 release managers can do anything on those release scripts if they don't touch branch-2.4.

[holdenk]

If this makes it harder for rolling a release yes lets revert. During the 2.4.6 RC process I ran into non-transient issues with the release process locally that hadn't shown up in Jenkins so we had some "junk" tags pushed and I was trying to avoid that case.

[cloud-fan]

I've reverted it from master/3.0, thanks for the understanding!