Skip to content

Decide how to handle canceling builds #1928

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

Closed
presleyp opened this issue May 31, 2022 · 14 comments
Closed

Decide how to handle canceling builds #1928

presleyp opened this issue May 31, 2022 · 14 comments
Labels
api Area: HTTP API needs decision Needs a higher-level decision to be unblocked. stale This issue is like stale bread.

Comments

@presleyp
Copy link
Contributor

presleyp commented May 31, 2022

Here are some notes on how we might implement @spikecurtis's idea of not exposing cancel jobs alone to the user, but rather always pairing them with start or stop jobs so that the workspace is always in a meaningful state. One potential issue is that the cancel job could succeed and the start or stop job fail, so the canceled state may not be totally avoidable.

Reasons a user might want to cancel a start job and buttons they want in that scenario (button label in bold and then API requests it would call:

  1. I changed my mind, I don't want to start right now. Cancel (issues cancel followed by stop)
  2. The job is hanging, I guess I can't start right now, so I don't want to waste resources, I want to stop. Cancel (cancel, stop)
  3. The job is hanging, but I'm feeling lucky, I want to try starting again. Retry or Restart (cancel, start)
  4. I made an edit, now I want to start with the edits included. Restart (Retry doesn't sound as good here) (cancel, start)

Reasons a user might want to cancel a stop job and buttons:

  1. I changed my mind, I don't want to stop right now. Cancel (cancel, start)
  2. The job is hanging, I guess I can't stop right now, leave me in a meaningful state. Cancel (cancel, start)
  3. The job is hanging, I want to try stopping again. Retry (Restop sounds weird) (cancel, stop)
@presleyp presleyp added the needs decision Needs a higher-level decision to be unblocked. label May 31, 2022
@spikecurtis
Copy link
Contributor

One potential issue is that the cancel job could succeed and the start or stop job fail, so the canceled state may not be totally avoidable.

I'd call that a failed job, same as if there were no cancelled job.

@spikecurtis
Copy link
Contributor

A "delete" also involves a job. Is this still considered a build/action? Can we cancel it?

@f0ssel
Copy link
Contributor

f0ssel commented May 31, 2022

What are the exact decisions needed here? I'm happy to try and weigh in.

@misskniss
Copy link

I would like to include when this decision should take effect. Is this something that we are trying to bring in for Community? If so, go ahead and label it for Community.

@misskniss misskniss added the api Area: HTTP API label Jun 1, 2022
@presleyp
Copy link
Contributor Author

presleyp commented Jun 1, 2022

@misskniss this is not needed for Community. I just wanted to create a place for the discussion to live.

@spikecurtis @f0ssel sorry, I stopped a little short because I realized this isn't a priority just yet.

Yes, delete has to be considered as well because it can also be canceled.

Spike, it sounds like you might be proposing a change to the API rather than just the frontend, where it sends back a failed job status if a double job (cancel then something) partially fails. That dovetails with Garrett's push to have the frontend do less derivation.

@ketang
Copy link
Contributor

ketang commented Jun 1, 2022

As long as we have cancellation functionality with a name that users can understand, we can take as long as we want. I think cancellation is necessary for Community, but perfectly-named cancellation is not.

@misskniss misskniss added this to the Community MVP milestone Jun 3, 2022
@misskniss misskniss mentioned this issue Jun 3, 2022
16 tasks
@misskniss misskniss removed this from the Community MVP milestone Jun 3, 2022
@misskniss
Copy link

Leaving this as is for Community. We can come back to it.

@Kira-Pilot
Copy link
Member

I just wanted to drop a gif of the resulting UI here (I think this is the right ticket) because I encountered this again today and thought the indeterminate state was a little confusing - you can see the user is presented with both start and stop options:
Kapture 2022-06-07 at 13 52 15

Regardless of how we choose to handle this on the BE, it would be nice to lend a little more clarity to the FE, maybe by hiding some buttons or something.

@ketang
Copy link
Contributor

ketang commented Jun 7, 2022

Oh yikes, having both start and stop is not good not at all.

@spikecurtis
Copy link
Contributor

Oh yikes, having both start and stop is not good not at all.

Wait, why? If I cancel a Start, why is it wrong to Start again? Why is it wrong to Stop?

@ketang
Copy link
Contributor

ketang commented Jun 8, 2022

If your workspace incarnation i is currently stopping, I think you shouldn't be able to hit the stop button again. It won't do anything. The presence of the active stop button implies that previous attempts to stop didn't take. I also think (for now) you shouldn't be able to spin up incarnation i+1 because we haven't validated the need nor designed a UI that can unconfusingly represent that.

@spikecurtis
Copy link
Contributor

Start / Stop / Delete are being shown after the Action is cancelled, not while the workspace is stopping.

I agree that if the Stop build action is currently pending/executing (both I believe are interpreted as Stopping in the UI), you can't Stop again. But if you Cancel the Stop, then you should be able to run a new Action with any transition.

@ketang
Copy link
Contributor

ketang commented Jun 9, 2022

In that situation you shouldn't be able to run Start, so it avoids the problem of having Start and Stop buttons simultaneously.

@github-actions
Copy link

github-actions bot commented Aug 9, 2022

This issue is becoming stale. In order to keep the tracker readable and actionable, I'm going close to this issue in 7 days if there isn't more activity.

@github-actions github-actions bot added the stale This issue is like stale bread. label Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Area: HTTP API needs decision Needs a higher-level decision to be unblocked. stale This issue is like stale bread.
Projects
None yet
Development

No branches or pull requests

6 participants