Opened 9 months ago

Closed 8 months ago

Last modified 8 months ago

#34768 closed Cleanup/optimization (fixed)

Calling `colorama.init()` on module load can have unwanted side effects

Reported by: Charlie DeTar Owned by: Sulabh Katila
Component: Core (Management commands) Version: 4.2
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

django.core.management.color calls `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 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 describes itself as providing a "simple cross-platform API for printing colored terminal text from Python"; and indeed, 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.

Change History (11)

comment:1 by Natalia Bidart, 9 months ago

Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

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.

comment:2 by Lufafa Joshua, 8 months ago

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.

comment:3 by Charlie DeTar, 8 months ago

@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:

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.

comment:4 by Lufafa Joshua, 8 months ago

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.

comment:5 by Sulabh Katila, 8 months ago

Owner: changed from nobody to Sulabh Katila
Status: newassigned

comment:6 by Mariusz Felisiak, 8 months ago

Has patch: set

comment:7 by Mariusz Felisiak, 8 months ago

Triage Stage: AcceptedReady for checkin

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 8 months ago

Resolution: fixed
Status: assignedclosed

In 9a9620d:

Fixed #34768 -- Avoided initializing colorama on non-Windows platforms.

comment:9 by Tim Graham, 8 months ago

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.

in reply to:  9 comment:10 by Mariusz Felisiak, 8 months ago

Replying to 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 PR.

comment:11 by GitHub <noreply@…>, 8 months ago

In 048d75a:

Refs #34768 -- Ignored lack of just_fix_windows_console() for colorama < 0.4.6.

Note: See TracTickets for help on using tickets.
Back to Top