Auto-cancel build when a new commit is pushed#8043
Auto-cancel build when a new commit is pushed#8043simon-mo merged 21 commits intoray-project:masterfrom
Conversation
ad3d480 to
6e97e8b
Compare
| head = git("symbolic-ref", "-q", "HEAD") | ||
| ref = git("for-each-ref", "--format=%(upstream:short)", head) | ||
| except subprocess.CalledProcessError: |
There was a problem hiding this comment.
This is leaky. when calling git, how do we know CalledProcessErorr will happen? Consider handle this in git implementation by catching something like this and return None?
There was a problem hiding this comment.
Sorry I'm confused what you mean here. CalledProcessError happens when git exits with a nonzero status. We need the status code in some cases (though not here) so I can't just return None. I'm not sure I understand what the problem is though -- this is the normal way to indicate a subprocess call was unsuccessful. What's leaky?
simon-mo
left a comment
There was a problem hiding this comment.
looks much better now! Can you write some description about how exactly does this script determines the new commits. In particular, explain how the following works together because they may be foreign to developers:
git symbolic-refgit for-each-refgit ls-remoteand the assumptions thatref/pullsorref/headswill contains the pull request number.
|
|
||
| def main(program, *args): | ||
| p = argparse.ArgumentParser() | ||
| p.add_argument("--skip_repo", action="append", help="Repo to exclude.") |
There was a problem hiding this comment.
Can we hard code this as a constant in this file?
There was a problem hiding this comment.
I was trying not to make the script specific to ray-project/ray... it might be useful elsewhere?
ci/remote-watch.py
Outdated
| return None | ||
|
|
||
|
|
||
| def git_branch_info(): |
There was a problem hiding this comment.
can we make this function simpler?
There was a problem hiding this comment.
I can try again, but it might take a while... I recall it was originally simple and I had to complicate it to handle all the different cases.
There was a problem hiding this comment.
I don't think I know how to simplify this :\ I recall it was extremely painful getting all the edge cases right... Travis vs. GitHub, merge commit vs. normal commit, single commit vs. multiple commits, etc.
|
Made some changes, here's confirmation that the code still works: |
|
Test FAILed. |
|
Test FAILed. |
|
Test FAILed. |
|
Test FAILed. |
This allows builds to self-terminate when they detect new commits on the remote.
For cases where this is intentional and desirable, this mechanism can be bypassed by keeping the remote branch alive, and putting a line like the following in the commit message:
A list of specific CI providers to keep alive can also be provided as follows:
Why are these changes needed?
Builds currently don't stop when remote branches are overwritten.
Checks
scripts/format.shto lint the changes in this PR.