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

Avoid generating 'No handlers could be found for logger "elasticsearch"' errors ... #169

Closed
wants to merge 3 commits into from

Conversation

jmehnle
Copy link

@jmehnle jmehnle commented Dec 23, 2014

@jmehnle
Copy link
Author

jmehnle commented Dec 23, 2014

This addresses #67.

@jmehnle
Copy link
Author

jmehnle commented Dec 23, 2014

Travis CI said:

  File "/home/travis/build/elasticsearch/elasticsearch-py/elasticsearch/__init__.py", line 19, in <module>
    logger.addHandler(logging.NullHandler())
AttributeError: 'module' object has no attribute 'NullHandler'

However, the logging module clearly defines a NullHandler class:

Python 2.7.9 (default, Dec 13 2014, 15:13:49)
[GCC 4.2.1 Compatible Apple LLVM 6.0 (clang-600.0.56)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import logging
>>> logger = logging.getLogger('elasticsearch')
>>> logger.addHandler(logging.NullHandler())
>>> ^D

The build should not be failing.

@jmehnle
Copy link
Author

jmehnle commented Dec 23, 2014

Duh. Travis CI uses Python 2.6 to build this module, and logging.NullHandler exists only since 2.7.

Python 2.6 does not define logging.NullHandler.
@honzakral
Copy link
Contributor

I am wondering why do you prefer the silencing approach instead of the default behavior. The message might be irritating for some people but I find it helps a lot of people discover that there is such a thing as logging and that our library uses it.

The HOWTO you linked specifies:

If the using application does not use logging, and library code makes logging calls, then (as described in the previous section) events of severity WARNING and greater will be printed to sys.stderr. This is regarded as the best default behaviour.

Thanks!

@jmehnle
Copy link
Author

jmehnle commented Dec 26, 2014

My main concern is the odd No handlers could be found for logger "elasticsearch" message, which has made my users investigate a possible code error. Look at it from the perspective of code that doesn't otherwise use the logging module: why would you require such code to import logging and register a NullHandler just to prevent an obscure error message? (Also, if people have a hard time discovering that the elasticsearch module uses the logging module, perhaps that could be better addressed in documentation, as the HOWTO also suggests?)

That said, your implicit reference to the Python 3 version of the HOWTO hints at Python 3.2+ behavior actually being different from 2.x/3.0/3.1 behavior:

In Python 3.2 and later, the behaviour is as follows:

  • The event is output using a ‘handler of last resort’, stored in logging.lastResort. This internal handler is not associated with any logger, and acts like a StreamHandler which writes the event description message to the current value of sys.stderr (therefore respecting any redirections which may be in effect). No formatting is done on the message - just the bare event description message is printed. The handler’s level is set to WARNING, so all events at this and greater severities will be output.

This means under Python 3.2+ a No handlers could be found for logger "elasticsearch" message will not be generated, which, again, is my main concern. Given that elasticsearch doesn't list Python 3.0 and 3.1 as supported platforms in setup.py, and Python 2.6's logging module doesn't define the NullHandler, would it be fair to limit the registration of the NullHandler to only Python 2.7?

@honzakral
Copy link
Contributor

I still don't honestly see the reason for adding this code just to avoid the No handlers could be found for logger "elasticsearch" message. Am I missing something?

The HOWTO explicitly states that providing no configuration is regarded as best default behavior and suggest adding the NullHandler only if you don't wish warnings to be outputted by default and the No handlers... message. I think that those messages are useful to our users so don't want to silence them, especially if it would mean adding (albeit trivial) code.

@jmehnle
Copy link
Author

jmehnle commented Dec 29, 2014

When you say "The HOWTO explicitly states that providing no configuration is regarded as best default behavior", then that's true only for Python 3.2+, since 3.2+ does not generate said error message. Python 2.x does, and the Python 2.7 HOWTO explicitly suggests a way for libraries to avoid generating said error message. If you can't agree with that, then I have nothing else to try to convince you. Thanks for taking the time to consider my concern anyway.

@honzakral
Copy link
Contributor

I found that most people don't share my view on this, thank you for your patience!

This was merged in 51c89a8

@honzakral honzakral closed this Dec 31, 2014
@jmehnle
Copy link
Author

jmehnle commented Dec 31, 2014

@honzakral, thanks for being accommodating about this! Happy New Year to you and @elasticsearch!

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.

None yet

2 participants