[Django] #34768: Calling `colorama.init()` on module load can have unwanted side effects
Django
-------------------------------------+-------------------------------------
Reporter: Charlie | Owner: nobody
DeTar |
Type: | Status: new
Uncategorized |
Component: Core | Version: 4.2
(Management commands) |
Severity: Normal | Keywords:
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
`django.core.management.color` calls
[https://github.com/django/django/blob/aa3cb3f37265be37d892e2b391ff023e9caee2a4/django/core/management/color.py#L12-L14
`colorama.init()`] on module load if the module is present in the system.
This causes `sys.stdout` to be replaced by an instance of
`colorama.ansitowin32.StreamWrapper`.
This, in turn, can cause problems in other contexts that do not expect
`sys.stdout` to be a colorama wrapper. For example, using `pytest` with
`pytest-xdist` in `--looponfail` mode (which uses
[https://pypi.org/project/execnet/ execnet] to further wrap its output)
results in ansi color escapes getting dropped from the output.
In discussion of #32740, a maintainer commented "There's no reason to have
colorama installed at all if you're not on windows". However, colorama
now [https://github.com/tartley/colorama describes itself] as providing a
"simple cross-platform API for printing colored terminal text from
Python"; and indeed, [https://github.com/Riverside-Healthcare/djlint
djlint] installs colorama as a dependency for all platforms, using other
color utilities within the project rather than the stream wrapper. In
short: it is probably not right to assume that `colorama` will only be
present on win32, and thus Django probably shouldn't be replacing
`sys.stdout` with it on module import without at a platform check.
To summarize: a linux environment which installs:
- Django v3.2 -> dev
- djlint v1.32.x
- pytest-xdist v3.3.x
will lose colorized output while running `pytest` with `--looponfail` due
to Django's call to `colorama.init()`, which is an unnecessary call on
that platform.
--
Ticket URL: <https://code.djangoproject.com/ticket/34768>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Type: | Status: new
Cleanup/optimization |
Component: Core (Management | Version: 4.2
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
Accepting on the basis that the comments in the original work adding
`colorama` (#31216, #32740) seem to indicate that there was an implicit
assumption that it was only used in windows, but we should
investigate/discuss how to best approach the solution.
--
Ticket URL: <https://code.djangoproject.com/ticket/34768#comment:1>
Django
-------------------------------------+-------------------------------------
Reporter: Charlie DeTar | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Core (Management | Version: 4.2
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Lufafa Joshua):
Yes Colorama provides a simple Cross Platform API and works on non windows
platforms, even though primarily intended for windows as the documentation
suggests https://pypi.org/project/colorama/, my simple suggestion in that
case would be to disable colorama's wrappers for non windows platforms by
passing {{{wrap=False}}} inside the {{{init()}}} method. Am not really
sure if that solves the problem for you [# Charlie DeTar ]. I personally
believe full support for non windows platforms is still a process under
way.
--
Ticket URL: <https://code.djangoproject.com/ticket/34768#comment:2>
Django
-------------------------------------+-------------------------------------
Reporter: Charlie DeTar | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Core (Management | Version: 4.2
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Charlie DeTar):
@LufafaJoshua passing `wrap=False` would solve my problem - but it would
likely break Django's usecase for windows, since it would then be
necessary for Django to write to `AnsiToWin32(sys.stdout).stream`
explicitly when it wants colorized output on windows. I believe the whole
reason for Django to use colorama at all is to keep colorized output on
windows consoles without having to explicitly switch or feature detect
them throughout the codebase.
Colorama's newer `colorama.just_fix_windows_console()` initializer (which
wasn't around when colorama was first added to Django) may be a better
fit. That will cause colorama to attempt to detect if the console it's
writing to is a windows console which does not support ANSI color
sequences, and wrap it only if that's the case. The caveat here is that
its detection may not be foolproof, so there could be edgecases for
particular windows console configurations that could lose colorized
output. In my not unbiased opinion, it'd be better to err on the side of
leaving `sys.stdout` alone, rather than erring on the side of always
replacing it (with the attendant edge case failures that can cause).
I managed to work around my issue by adding a `conftest.py` file with the
following hook to de-initialize colorama while pytest is loading, and
monkeypatch it to prevent subsequent initialization:
{{{
#!python
def pytest_configure():
"""
Hack to retain colorized output in pytest-xdist, working around this
bug:
https://code.djangoproject.com/ticket/34768
"""
try:
import colorama
# Undo any previously called colorama.init()
colorama.deinit()
# Prevent any future call to colorama.init() from having an effect
colorama.init = lambda: None
except ImportError:
pass
}}}
I imagine any edge-case windows console users whose configuration isn't
properly detected by `just_fix_windows_console()` could add a similar hack
(but in reverse) to force a call to `colorama.init()` instead if they
wished to restore colorized output. That, or open a ticket with colorama
to improve feature-detection of their configuration.
--
Ticket URL: <https://code.djangoproject.com/ticket/34768#comment:3>
Django
-------------------------------------+-------------------------------------
Reporter: Charlie DeTar | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Core (Management | Version: 4.2
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Lufafa Joshua):
Hello [# Charlie DeTar], thanks a lot for your feedback, and i personally
agree with the fact that there could be undesired behavior with calling
{{{colorama.init()}}}
on non windows platforms for example
https://github.com/tartley/colorama/issues/209 and that could be one
reason to deinitialize it for non windows users.
--
Ticket URL: <https://code.djangoproject.com/ticket/34768#comment:4>
Django
-------------------------------------+-------------------------------------
Type: | Katila
Cleanup/optimization | Status: assigned
Component: Core (Management | Version: 4.2
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* owner: nobody => Sulabh Katila
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/34768#comment:5>
Django
-------------------------------------+-------------------------------------
Reporter: Charlie DeTar | Owner: Sulabh
Type: | Katila
Cleanup/optimization | Status: assigned
Component: Core (Management | Version: 4.2
commands) |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/17211 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/34768#comment:6>
Django
-------------------------------------+-------------------------------------
Reporter: Charlie DeTar | Owner: Sulabh
Type: | Katila
Cleanup/optimization | Status: assigned
Component: Core (Management | Version: 4.2
commands) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Mariusz Felisiak):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/34768#comment:7>
Django
-------------------------------------+-------------------------------------
Reporter: Charlie DeTar | Owner: Sulabh
Type: | Katila
Component: Core (Management | Version: 4.2
commands) |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"9a9620dda649dcdb685662bbd9cb409bfeff4214" 9a9620d]:
{{{
#!CommitTicketReference repository=""
revision="9a9620dda649dcdb685662bbd9cb409bfeff4214"
Fixed #34768 -- Avoided initializing colorama on non-Windows platforms.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34768#comment:8>
Django
-------------------------------------+-------------------------------------
Reporter: Charlie DeTar | Owner: Sulabh
Type: | Katila
Cleanup/optimization | Status: closed
Component: Core (Management | Version: 4.2
commands) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Tim Graham):
The build for django-snowflake runs on GitHub actions with ubuntu-latest.
It seems that colorama 0.4.4 is preinstalled and thus after this change
the build fails with `module 'colorama' has no attribute
'just_fix_windows_console'`. I think this situation should be handled
gracefully so that third-party package developers aren't required to
upgrade colorama as a part of their builds.
--
Ticket URL: <https://code.djangoproject.com/ticket/34768#comment:9>
Django
-------------------------------------+-------------------------------------
Reporter: Charlie DeTar | Owner: Sulabh
Type: | Katila
Cleanup/optimization | Status: closed
Component: Core (Management | Version: 4.2
commands) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by Mariusz Felisiak):
Replying to [comment:9 Tim Graham]:
> The build for django-snowflake runs on GitHub actions with ubuntu-
latest. It seems that colorama 0.4.4 is preinstalled and thus after this
change the build fails with `module 'colorama' has no attribute
'just_fix_windows_console'`. I think this situation should be handled
gracefully so that third-party package developers aren't required to
upgrade colorama as a part of their builds.
Agreed, check out [https://github.com/django/django/pull/17218 PR].
--
Ticket URL: <https://code.djangoproject.com/ticket/34768#comment:10>
Django
-------------------------------------+-------------------------------------
Reporter: Charlie DeTar | Owner: Sulabh
Type: | Katila
Cleanup/optimization | Status: closed
Component: Core (Management | Version: 4.2
commands) |
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by GitHub <noreply@…>):
In [changeset:"048d75aeb1e2b1c08b1b9ec359397f00aec1b57d" 048d75a]:
{{{
#!CommitTicketReference repository=""
revision="048d75aeb1e2b1c08b1b9ec359397f00aec1b57d"
Refs #34768 -- Ignored lack of just_fix_windows_console() for colorama <
0.4.6.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/34768#comment:11>