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

Disable "Terminate" button when there's unprocessed signals #123

Closed
wants to merge 2 commits into from

Conversation

eagletmt
Copy link
Member

To prevent multiple clicks on "Terminate" button.

@cookpad/dev-infra please review

@@ -7,7 +7,7 @@ def index
end

def destroy
if @execution.try!(:pid)
if @execution.terminatable?
Copy link
Contributor

Choose a reason for hiding this comment

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

Checking terminatable(L10) and creating an execution record(L13) are not atomic operations. Is it acceptable?

Copy link
Member Author

Choose a reason for hiding this comment

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

added a unique index to prevent duplication

@eagletmt eagletmt force-pushed the dedup-terminate-requests branch from c6fb660 to 622fcfe Compare March 18, 2019 04:25
@eagletmt
Copy link
Member Author

@cookpad/infra please review

@itkq
Copy link

itkq commented Jun 3, 2019

@eagletmt I'm going to merge #128 and release it as v0.5.2. This PR seems merge-ready, so do you think it is good to merge and ship this at the same time?

@eagletmt eagletmt closed this Jul 16, 2020
@eagletmt eagletmt deleted the dedup-terminate-requests branch July 16, 2020 08:33
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.

5 participants