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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Request: Allow maintainers to cancel PR builds #7660

Closed
pllim opened this issue Nov 11, 2020 · 13 comments · Fixed by #8850
Closed

Feature Request: Allow maintainers to cancel PR builds #7660

pllim opened this issue Nov 11, 2020 · 13 comments · Fixed by #8850
Assignees
Labels
Feature New feature Needed: design decision A core team decision is required

Comments

@pllim
Copy link
Contributor

pllim commented Nov 11, 2020

Details

cc @bsipocz @astrofrog @Cadair

What is happening right now

We really like the PR build feature, however there is a missing feature that we sorely need. When a contributor pushes a bunch of commits to a PR within a short time frame, we ended up with a long queue in the build for the same PR. We are unable to manually cancel out-of-date jobs, leaving only the one running from the latest commit in the PR to complete.

This resulted in the waiting game. This is also not very good financially for you nor environmentally for everyone else.

What we want to happen

A "cancel job" button for every build in https://readthedocs.org/projects/astropy/builds/ that is running. This button at the very least should be accessible to all RTD admins in the project.

Nice to have: Let the PR author able to cancel builds from their own PR.

@stsewd stsewd added Feature New feature Needed: design decision A core team decision is required labels Nov 11, 2020
@pllim
Copy link
Contributor Author

pllim commented Nov 11, 2020

From @Cadair:

Nice to have also: Automatically cancel build for a PR if a new build from the same PR has superseded it.

@pllim
Copy link
Contributor Author

pllim commented Nov 11, 2020

p.s. I also think #871 complements this feature.

@humitos
Copy link
Member

humitos commented Nov 12, 2020

Thanks for opening this issue.

I wanted to chime in here to comment some history related to this. Some time ago we implemented "De-duplication of builds": so, if you trigger 2+ builds almost together for the same RTD version, they are marked as duplicated and only one is ran.

In another set of work, we tried to implement "Cancel a running build", but we found some complications and we ran out of time for that task and considering that it wasn't too requested, we decided to not keep implementing it at that time.

However, lot of users already asked for the ability to "Cancel a build" in 2016 but the main reason at that time was because the stuck on some state (because RTD failed with an unknown reason) and the state never got updated. We created a task that clear those builds if it detects they are stale.

I think the ability to "Cancel a build" is still valid and should be implemented at some point. IMO, the best way to implement this will be to use Celery internals as described in #3984 and #7031. The main blocker here is the lack of the time required to implement this. It's a re-write of a lot of RTD internal building system and will require lot of testing as well.

@marscher
Copy link

Skipping a build would also be a nice option. E.g. parsing commit messages like other CI services for "[ci skip]" or "[doc skip]".

@CagtayFabry
Copy link

another approach to maybe lighten the load of PR builds (which are awesome btw) might be to optionally discard builds for draft PRs when the PR option is enabled.

This info is easy to parse directly from the GH event data and gives the users some control over which Docs to build or not.

@astrojuanlu
Copy link
Contributor

Summary of ideas collected here in light of #8772 happening again, sorted by how late the cancel action happens:

  • Avoid queuing builds from draft PRs
  • Skip builds by reading certain strings from the commit message, like [docs skip] Can i mark a commit as to skip building the docs? #871
  • Button to cancel a queued build that hasn't started yet (i.e. remove it from the queue)
  • Button to cancel a running build (probably the most difficult one)

@pllim
Copy link
Contributor Author

pllim commented Mar 1, 2022

Hello. What should I expect from the dashboard now that my request is marked as resolved? I don't see any cancel button still.

@astrojuanlu
Copy link
Contributor

@pllim The issue was closed, but most likely not deployed to production yet.

@pllim
Copy link
Contributor Author

pllim commented Mar 1, 2022

Is there a timeline for deployment so I can keep an eye out? I am looking forward to this feature, so thank you very much! 😸

@humitos
Copy link
Member

humitos commented Mar 1, 2022

We are doing the deploy right now 😄

@pllim
Copy link
Contributor Author

pllim commented Mar 1, 2022

I see the button now. Thank you, all! 🙏

@marscher
Copy link

marscher commented Mar 2, 2022

Thank you so much for taking a shot at this. Is it also possible to trigger "the button" with some sort of an API, so we could cancel old builds automatically for instance?

@humitos
Copy link
Member

humitos commented Mar 2, 2022

Is it also possible to trigger "the button" with some sort of an API, so we could cancel old builds automatically for instance?

@marscher this is not possible for now. However, we plan to automatically cancel old builds, see #8961. Please, if you have more use cases for that, comment on that issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature Needed: design decision A core team decision is required
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants