log: Fix color logging detection #2019
Conversation
tornado/log.py
Outdated
| return color | ||
| return True | ||
| elif colorama: | ||
| if sys.stderr is getattr(colorama, 'wrapped_stderr', object()): |
There was a problem hiding this comment.
Will getattr(colorama, 'wrapped_stderr') ever work? It doesn't look at first glance that colorama/__init__.py contains wrapped_stderr: https://github.com/tartley/colorama/blob/master/colorama/__init__.py
There was a problem hiding this comment.
Hmm, doesn't look like it, but this is what you had in the previous PR. I'm trying to dust off a windows VM to test this. Looks like this will probably need to do a different check like you have in #2014. You've changed there from looking at colorama.initialize.wrapped_stderr to an isinstance check; do you have a particular reason to prefer one form over another? Neither appears to be documented.
There was a problem hiding this comment.
Neither appear to be documented, which is what makes me think that using isinstance (and failing gracefully when there is an AttributeError there) is somewhat better. It also makes it a bit easier to limit it only to Windows, which is the only time we would need to resort to colorama (or otherwise reimplement exactly what colorama already does).
Yes, what you have was in my previous PR. I honestly don't know how that got by. This is one reason I explicitly included the demo in #2014 so that it's at least possible to interactively test.
There was a problem hiding this comment.
OK. Well, I'll go ahead and merge this so we can get a fix out the door, and see what happens with tartley/colorama#128.
The previous logic would fail whenever curses is missing (which is always true on windows, and is occasionally true on other platforms) and colorama is installed but not initialized in this process (this is a common occurrence for users of jupyter and ipython on windows). Fixes tornadoweb#2013 Fixes tornadoweb#2015
|
Were you able to run the demo in #2014 with this on the Windows VM? It would be good to double check that it doesn't still error. |
|
I didn't run the script from #2014 but I did some manual testing. |
The previous logic would fail whenever curses is missing (which is
always true on windows, and is occasionally true on other platforms)
and colorama is installed but not initialized in this process (this is
a common occurrence for users of jupyter and ipython on windows).
Fixes #2013
Fixes #2015
cc @mivade