-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Bugfix: Prevent continuous restart, if a dependency wasn't installed #35339
Bugfix: Prevent continuous restart, if a dependency wasn't installed #35339
Conversation
@@ -81,14 +81,14 @@ def suicide_when_without_parent(parent_pid): | |||
try: | |||
minion = salt.cli.daemons.Minion() | |||
minion.start() | |||
except (ImportError, SystemExit) as exc: | |||
log.error(exc) |
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.
We no longer want to set the restart flag to False here if we hit a SystemExit?
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.
@cachedout it is already False. We do not want set it to 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.
Ah, I didn't read up enough. I do like it being explicit but you're right. Thanks!
@cachedout This is not my lint problem, as this code is already there. Should I extra-fix this in this PR? |
I think it's complaining because L86 is now an ancestor to what you added on L84, so yes, this was caused by your change. If you could please clean it up then we'll get this in. Thanks. :] |
590ff2c
to
12af60b
Compare
@cachedout here you go. 😉 |
What does this PR do?
Maintenance bugfix.
What issues does this PR fix or reference?
Prevents minion restart forever if a dependency module wasn't installed by a various reasons.
Tests written?
No