-
Notifications
You must be signed in to change notification settings - Fork 12
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: DPE-6485 optional restart on instance configuration #607
base: main
Are you sure you want to change the base?
Conversation
logger.info("Recovering unit") | ||
if self.app.planned_units() == 1: | ||
self._mysql.reboot_from_complete_outage() | ||
else: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A small improvement to reduce indentation levels here would be to add a return
clause within the if
statement, and leave everything within the else
block un-indented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, one curiosity question.
@@ -1955,6 +1961,7 @@ def is_instance_in_cluster(self, unit_label: str) -> bool: | |||
user=self.server_config_user, | |||
password=self.server_config_password, | |||
host=self.instance_def(self.server_config_user), | |||
exception_as_warning=True, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please share some explanation here? Tnx!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the method is called on an instance that does not belong to the cluster (initialization, some self healing cases), it throws an noisy error log which is not a failure per se, but a valid output.
…re/optional-restart-on-configure
Issue
Instance configuration need to expose
restart
for explicit restart under pebble.Solution
restart