boto3 icon indicating copy to clipboard operation
boto3 copied to clipboard

Use logging.NullHandler

Open aguckenber-chwy opened this issue 1 year ago • 3 comments

Describe the feature

Referencing this line. Please change it to using logging.NullHandler() instead of a custom handler. Because boto3 (as well as botocore and s3transfer) creates its own NullHandler, I as a developer can't iterate through the handlers and do a simple isinstance(handler, logging.NullHandler).

Use Case

Need to pull the class name and hope boto3 stays using the name NullHandler:

```python
loggers = []
for logger_name, logger_instance in logging.root.manager.loggerDict.items():
    if hasattr(logger_instance, "handlers"):
        for handler in logger_instance.handlers:
            if handler.__class__.__name__ != "NullHandler": # isinstance is preferred here
                loggers.append(logger_name)
                break

Proposed Solution

logging.getLogger('boto3').addHandler(logging.NullHandler())

And remove the custom class. Or, bare minimum, at least inherit from pythons NullHandler instead of from Handler.

Other Information

No response

Acknowledgements

  • [X] I may be able to implement this feature request
  • [ ] This feature might incur a breaking change

SDK version used

latest

Environment details (OS name and version, etc.)

not-needed

aguckenber-chwy avatar Aug 29 '22 17:08 aguckenber-chwy

Thanks @aguckenber-chwy for the feature request. I brought this up for discussion with the team and it's a change that they may be open to considering. Would you be willing to create a PR for this?

Just to elaborate a bit more on this issue - the approach you're suggesting is recommended here: https://docs.python.org/3/howto/logging.html#configuring-logging-for-a-library. But NullHandler was not available prior to Python 2.7 so wasn't originally used. We do have some concerns about breaking customers who may rely on the current logging behavior.

tim-finnigan avatar Aug 30 '22 17:08 tim-finnigan

@tim-finnigan Before I do this, does your team also work with botocore and s3transfer? I noticed both those libraries have the issue as well so I may PR those as well or I may reach out to those separately via a feature request.

As for the concern about breaking backwards compatibility, the safest bet if that is a concern would be to make sure NullHandler is explicitly imported

This is the simplest but could cause potential problems if costumers use from boto3 import NullHandler

... remove custom class
logging.getLogger('boto3').addHandler(logging.NullHandler())

This should be fine since users can still do from boto3 import NullHandler as they did.

from logging import NullHandler

... Remove custom class

logging.getLogger('boto3').addHandler(NullHandler())

This should prevent any breaking behaviors for Python3 users. Or at least keep it to a minimum. Python2.7 would obviously still have issues but given that is no longer supported by Python nor AWS Lambda, I would imagine that isn't much of a concern for your team. What are your thoughts?

aguckenber-chwy avatar Aug 30 '22 17:08 aguckenber-chwy

Hi @aguckenber-chwy thanks for following up. Yes our team owns the botocore and s3transfer repositories as well. Also support for Python 3.6 and earlier is deprecated as noted here so issues with Python 2.7 shouldn't be a concern.

But we still can’t say for sure if this change will be considered due to the possibility of breaking customers. The case we'd be concerned about is any logging adjustments based off our class. If we are no longer registering that handler, some customers' code will break because it doesn't exist. So for now I think we should leave this issue open for others to weigh in and provide feedback.

tim-finnigan avatar Sep 06 '22 17:09 tim-finnigan