Skip to content

Conversation

@leifj
Copy link
Contributor

@leifj leifj commented Jan 17, 2019

While working on the pyFF refactor it became clear that logging should improve a bit. A simple low-hanging fruit is to enable project-local loggers so you can filter logs. This doesn't affect anyone not using a config-driven log setup but enables stuff like pyramids to work better. I also locked down idna because httplib2 blows up otherwize for py2 - we can drop this later when we drop py2 compat.

All Submissions:

  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you added an explanation of what problem you are trying to solve with this PR?
  • Have you added information on what your changes do and why you chose this as your solution?
  • Have you written new tests for your changes?
  • Does your submission pass tests?
  • This project follows PEP8 style guide. Have you run your code against the 'flake8' linter?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 73.201% when pulling 52aa6da on leifj:master into d724f8d on IdentityPython:master.

DS = ElementMaker(namespace=NS['ds'], nsmap=NSDefault)


log = logging.getLogger('xmlsec')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The convention is to use

logger = logging.getLogger(__name__)

__name__ is the module’s name in the Python package namespace.

Either way, I think it's fine.

Copy link
Contributor

@johanlundberg johanlundberg Jan 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also be happier if __name__ was used instead of explicit logger name and if the variable named log was renamed to logger, but no real objections.

@leifj leifj merged commit 568d81c into IdentityPython:master Jan 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants