-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: update to plugin reconnect mechanics #372
Conversation
f64e14d
to
9011063
Compare
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.
Just questions - LGTM
# Disable all removed plugins and stop any active tasks they may be running. | ||
for plugin in removed: | ||
logger.info( | ||
'registered plugin not found during refresh, marking as disabled', |
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.
Is this a logger.warn/error? (Maybe/Maybe not?)
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.
updated in latest commit
# is nothing to do here. | ||
for plugin in existing: | ||
if plugin.disabled: | ||
logger.info( |
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.
+1 (disable should be disabled in the log statement)
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.
good catch - thanks!
This is probably a typo causing the build error: ModuleNotFoundError: No module named 'importlib_metadata' |
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!
updated per the reviews, but I'm still cycling a bit on this one since it looks like there is some weird python dependency stuff going on between different versions (3.6 vs 3.8) which is causing weirdness in CI |
a8078b3
to
644d27a
Compare
This PR:
fixes #371