Skip to content

Make Lifecycle ignore repeated stop requests#28

Merged
xvrl merged 1 commit intomasterfrom
idempotentLifecycle
Nov 12, 2015
Merged

Make Lifecycle ignore repeated stop requests#28
xvrl merged 1 commit intomasterfrom
idempotentLifecycle

Conversation

@drcrallen
Copy link
Copy Markdown
Contributor

Related to apache/druid#1627

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be an error? I think unlike stop, there's no good reason for start to be called multiple times

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Maybe? Prior behavior did not throw an error, which is why I did not do so here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel like it makes sense to either not change the behavior, or to have it throw an error; silently skipping over double starts seems sketchy. At least the old behavior would probably blow up in some way and let us know something was being done wrong.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@drcrallen drcrallen force-pushed the idempotentLifecycle branch from 639ad15 to ae0a6af Compare October 26, 2015 23:35
@drcrallen drcrallen changed the title Make Lifecycle ignore repeated stop/start requests Make Lifecycle ignore repeated stop requests Oct 26, 2015
@drcrallen drcrallen force-pushed the idempotentLifecycle branch from ae0a6af to 24765c5 Compare October 26, 2015 23:36
@himanshug
Copy link
Copy Markdown
Contributor

👍

@drcrallen
Copy link
Copy Markdown
Contributor Author

Bouncing for travis

@drcrallen drcrallen closed this Nov 11, 2015
@drcrallen drcrallen reopened this Nov 11, 2015
@xvrl
Copy link
Copy Markdown
Contributor

xvrl commented Nov 11, 2015

@drcrallen rebase on current master, it will help with travis

@drcrallen
Copy link
Copy Markdown
Contributor Author

ding! winner

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.

4 participants