-
Notifications
You must be signed in to change notification settings - Fork 296
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
ENH: lib/bot: fix behavior for unconfigured bots #2054
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #2054 +/- ##
===========================================
- Coverage 76.06% 76.04% -0.02%
===========================================
Files 434 434
Lines 23210 23218 +8
Branches 3100 3104 +4
===========================================
+ Hits 17654 17657 +3
- Misses 4849 4852 +3
- Partials 707 709 +2
|
intelmq/lib/bot.py
Outdated
setattr(self, option, value) | ||
elif option not in IGNORED_SYSTEM_PARAMETERS: | ||
self.logger.warning('Ignoring disallowed system parameter %r.', | ||
option) |
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.
Whats the difference between an option
that is not in ALLOWED_SYSTEM_PARAMETERS
and an option that is in IGNORED_SYSTEM_PARAMETERS
(besides the latter is logged)? Why not simply drop IGNORED_SYSTEM_PARAMETERS
and log all parameters that are not in ALLOWED_SYSTEM_PARAMETERS
?
Nitpicking: I prefer iterating through dicts using key, value
instead of option, value
. If you don't want to use key
I'd prefer the term parameter
instead of option
to make it more consistent.
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.
Ad naming: I just used the same naming as in the iteration below
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.
IGNORED_SYSTEM_PARAMETERS
are silently ignored, as they are legit, but irrelevant here.
Everything which is not in ALLOWED_SYSTEM_PARAMETERS
or IGNORED_SYSTEM_PARAMETERS
is not allowed and likely a user configuration error (e.g. things that should have gone into parameters
)
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.
Fixed the naming
894df82
to
488d885
Compare
e278acf
to
68d034e
Compare
Add `enabled` and `run_mode` as members of the `Bot` class with default values. Log the system parameters in the same way as default and runtime parameters. Warn if disallowed system parameters are encountered. Warn if no configuration could be found in the runtime configuration file for the bot's ID. Re-initialize logging if logging parameters have been changed by environment variables.
68d034e
to
0c92949
Compare
Add
enabled
andrun_mode
as members of theBot
class with default values.Log the system parameters in the same way as default and runtime parameters.
Warn if disallowed system parameters are encountered.
Warn if no configuration could be found in the runtime configuration file for the bot's ID.
Re-initialize logging if logging parameters have been changed by environment variables.