Logging Overhaul #947
Draft
Logging Overhaul #947
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DeepCode's analysis on #ef115d found:
Top issues
👉 View analysis in DeepCode’s Dashboard | Configure the bot |
amrsoll
commented
Aug 18, 2020
|
|
||
| # Disable the exception stacktrace AXEL : but why? | ||
| @force_kwargs(analytics=True, exc_info=False,) | ||
| def exception(self, msg, analytics=True, exc_info=False, *args, **kwargs): |
There was a problem hiding this comment.
I kept that because of the original behaviour, but is it necessary?
| date = self._getDateString() | ||
| id = kwargs.pop('id', self.id_short) | ||
| date = MrbLogger._getDateString() | ||
| id = id or self.name_short # FIXME id is reserved |
There was a problem hiding this comment.
id should really be refactored to something else
| else: | ||
| self.logger.error('Could not write exception to analytics, the analytics handler was not initialized.') | ||
| self.error('Could not write exception to analytics, the analytics handler was not initialized.', analytics=False) |
There was a problem hiding this comment.
prevents loops in case the analytics failed to load
|
I would like to merge this AFTER the Beta release, so I'm keeping it as a draft for now |
|
Latest update : Need to ask Gina about the plugin initialisation, and if the dirty MrBeamLogger is used everywhere. |
Closed
ahmed-mrbeam
force-pushed
the
develop
branch
2 times, most recently
from
202baee to
bcd6b27
Compare
ahmed-mrbeam
force-pushed
the
develop
branch
2 times, most recently
from
c49d8eb to
5d6659c
Compare
ahmed-mrbeam
force-pushed
the
develop
branch
2 times, most recently
from
1f28493 to
6c61b35
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The MrbLogger doesn't use a Logger anymore, because it is one. A lot has been delegated to the logging utilities themselves.
I do like the
force_kwargsfunction I wrote if I do say so myself :P It allows to get rid of the hackykwargs['some_key'] = kwargs.get('some_key', default_value)and averything is in the function definition instead, making it easier for the IDE and linters alike.Anyway, this also allows us to use
logging.getLoggerormrb_logger.getLoggerinterchangeably - they are essentially the same thing. The default Logger class has been set toMrbLoggeras soon as the plugin is loaded in, and theMrbLoggeris compatible with the standard use of Loggers