Skip to content
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

add get_logger and get_extended_debug_logger #170

Merged

Conversation

@pipermerriam
Copy link
Member

commented Aug 29, 2019

What was wrong?

The metaclass approach to installing loggers is nice but it turns out it's really hard to use because it doesn't play well with other metaclasses and you have to do a a lot of really gross boilerplate to make it work.

How was it fixed?

This PR exposes two new utilties, get_logger and get_extended_debug_logger.

get_logger is like logging.getLogger but it takes an additional logger_class which dictates the type of logger that should be returned.

get_extended_debug_logger is just an implementation of get_logger that always returns an ExtendedDebugLogger.

Cute Animal Picture

3b825e5d29c1d4bfb4570da4f689fede--pet-halloween-costumes-pet-costumes

@pipermerriam pipermerriam requested a review from carver Aug 29, 2019

@pipermerriam pipermerriam force-pushed the pipermerriam:piper/add-get_extended_debug_logger branch 4 times, most recently from 1ac5f94 to 6eb635f Aug 29, 2019

else:
with _use_logger_class(logger_class):
if name in logging.Logger.manager.loggerDict:
del logging.Logger.manager.loggerDict[name]

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Sep 5, 2019

Author Member

I'm adding a comment to explain why this is necessary.

@carver
carver approved these changes Sep 5, 2019
Copy link
Collaborator

left a comment

Yeah, looks great. Only think I can think to add is making sure that the context manager reset back to the default logger doesn't regress. Something like:

get_logger('a', SpecialLogger)

# make sure that the default logger doesn't switch to the special logger:
b = get_logger('b')
isinstance(b, logging.Logger)
@carver

This comment has been minimized.

Copy link
Collaborator

commented Sep 5, 2019

Hm, not sure why it's trying to run a doctest environment that doesn't exist. Maybe the tox test changed recently and the PR needs a rebase against the latest master?

@pipermerriam pipermerriam force-pushed the pipermerriam:piper/add-get_extended_debug_logger branch from 9d3ee2e to 79129bd Sep 5, 2019

pipermerriam added 2 commits Sep 5, 2019

@pipermerriam pipermerriam merged commit 56ef0b4 into ethereum:master Sep 5, 2019

8 checks passed

ci/circleci: docs Your tests passed on CircleCI!
Details
ci/circleci: lint-py36 Your tests passed on CircleCI!
Details
ci/circleci: lint-py37 Your tests passed on CircleCI!
Details
ci/circleci: py35 Your tests passed on CircleCI!
Details
ci/circleci: py36 Your tests passed on CircleCI!
Details
ci/circleci: py36-typing1 Your tests passed on CircleCI!
Details
ci/circleci: py37 Your tests passed on CircleCI!
Details
ci/circleci: pypy3 Your tests passed on CircleCI!
Details

@pipermerriam pipermerriam deleted the pipermerriam:piper/add-get_extended_debug_logger branch Sep 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.