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

Fix action button behavior #232

Merged
merged 1 commit into from
Aug 7, 2023
Merged

Fix action button behavior #232

merged 1 commit into from
Aug 7, 2023

Conversation

jbeisen
Copy link
Collaborator

@jbeisen jbeisen commented Aug 7, 2023

Enable the button even if an action is in progress.

Enable the button even if an action is in progress.
@mwylde
Copy link
Member

mwylde commented Aug 7, 2023

I don't think this is quite the right change. There are some states that are interruptible, and some that aren't, and all of this (including which action you can take) is meant to be determined by the server and sent back via the action_ properties.

That logic was here:

let (action_text, action, in_progress) = match (state.as_ref(), running_desired) {

You can see for example this line:

("Scheduling", true) => ("Stop", Some(Checkpoint), InProgress),

which is supposed to allow you to stop a scheduling job. It returns Some(Checkpoint) for the action, so it shouldn't be null.

@jbeisen
Copy link
Collaborator Author

jbeisen commented Aug 7, 2023

Yeah, I didn't change that logic in the API, just moved it into a function so it could be used by both the rest api and grpc api. But in 9c5de6e I added a condition to the console that disabled the button if an action is in progress, which I shouldn't have done.

Copy link
Member

@mwylde mwylde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense!

@jbeisen jbeisen merged commit a1739bd into master Aug 7, 2023
@jbeisen jbeisen deleted the fix-action-button branch August 7, 2023 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants