Skip to content

Conversation

@aledsage
Copy link
Contributor

Copy link
Member

@tbouron tbouron left a comment

Choose a reason for hiding this comment

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

I know this is marked as WIP, but I tested it and it failed for 2 reasons:

  • the missing return (see my comment)
  • the expected state set to on-fire where it should be stopped. There is a discussion to fix this on the mailing list

But once those are fixed, it should be good to go 👍

return;
}
if (isEntityStopping()) {
highlightViolation("Failure detected but entity stopping");
Copy link
Member

Choose a reason for hiding this comment

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

Should we highlight the violation here? I guess the question is more: is it really a violation when we are in a stopping state?

}
if (isEntityStopping()) {
highlightViolation("Failure detected but entity stopping");
LOG.info("Entity stopping, so ServiceRestarter not acting on failure detected at "+entity+" ("+event.getValue()+")");
Copy link
Member

Choose a reason for hiding this comment

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

You are missing a return;

@tbouron
Copy link
Member

tbouron commented Nov 22, 2017

@aledsage Do you think you can look at my comments above?

@nakomis
Copy link
Contributor

nakomis commented Nov 26, 2019

@aledsage Can you take a look at this to see if it is still relevant, and address @tbouron 's comment

Thanks

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.

3 participants