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

Option isatty=True not always working #84

Closed
pascal-hari opened this issue Mar 6, 2020 · 5 comments
Closed

Option isatty=True not always working #84

pascal-hari opened this issue Mar 6, 2020 · 5 comments

Comments

@pascal-hari
Copy link

Detected issue

When passing the parameter isatty=True to the method coloredlogs.install() the logger does not always generate a colored output.

Conditions

I use this option because I run on windows and use a git bash terminal. Since git bash is using pipes it's not detecting the terminal correctly with sys.stdout.isatty().

Expected behavior

I would expect with the isatty=True to the method coloredlogs.install() that the colored output is forced independent of sys.stdout.isatty().

Version

This has been discovered when running on version 14.0.

More information on issue

Debugging the module has shown that the method terminal_supports_colors(stream) returns false and the use_colors gets deactivated. More debugging has shown that in there the sys.stdout.isatty() is called.
Looking at those lines there is no difference from 'isatty'=None and 'isatty'=True.

Possible fix

For me changing line 433 to if use_colors is None: solved the issue. But I cannot tell if this is a valid fix, since I do not know other use cases.

@dwardzinski-jsf
Copy link

dwardzinski-jsf commented Mar 17, 2020

I am also having this issue. Looking at the source code, it would appear that only isatty=False has any effect (forces colors to be disabled). As you mentioned, isatty=True and isatty=None both use terminal auto-detection, so setting isatty=True makes no difference. Your fix (changing line 433 to if use_color is None:) would appear to be a valid fix, since it does make sense for isatty=True to force coloring regardless of whether the script thinks it's in an interactive terminal or not.

As for use cases, I've personally been having some issues with this since I'm calling my script through a job scheduler like cron (which disables coloring since it's not being called through a terminal). I've managed to find a workaround without changing the source code by making the scheduler call script -q -c 'python3 script.py' instead, which tricks it into thinking it's being called through a terminal. However, that also ended up causing other issues like SIGTERM signals not being intercepted properly (which is being used in my script). So, changing the source code looks like the way to go until this is fixed.

heni added a commit to heni/python-coloredlogs that referenced this issue Mar 20, 2020
@heni
Copy link

heni commented Mar 20, 2020

use could install my version locally, just executing command
pip install -U git+https://github.com/heni/python-coloredlogs.git@fix/isatty-force-attribute

@michael-o
Copy link

I basically have the same problem, but with a different usecase. My Python code runs a shared GitLab runner and I do pass isatty=True and expected that colors are forced even if we are not attached to a terminal, but still autodetection works. I'd expect that True would force/override detection.

@michael-o
Copy link

@heni Your patch works for me in the GitLab runner.

@xolox
Copy link
Owner

xolox commented Dec 10, 2020

Hi all and thanks for the feedback. I've just published coloredlogs release 14.1 which fixes this issue.

@xolox xolox closed this as completed Dec 10, 2020
bors bot added a commit to duckinator/bork that referenced this issue Dec 10, 2020
220: Update coloredlogs to 15.0 r=duckinator a=pyup-bot


This PR updates [coloredlogs](https://pypi.org/project/coloredlogs) from **14.0** to **15.0**.



<details>
  <summary>Changelog</summary>
  
  
   ### 15.0
   ```
   ----------------------------

Don&#39;t enable system logging on MacOS and Windows anymore.

This is backwards incompatible (which is why I&#39;m bumping the major version
number) however the old behavior has been reported to be rather problematic
(see :func:`~coloredlogs.syslog.is_syslog_supported()` for details) so this
seems like the best choice.

.. _Release 15.0: xolox/python-coloredlogs@14.2...15.0
   ```
   
  
  
   ### 14.3
   ```
   ----------------------------

Merged pull request `89`_ which enhances :func:`coloredlogs.install()` to
preserve the filters on handlers that are replaced by :pypi:`coloredlogs`.

.. _Release 14.3: xolox/python-coloredlogs@14.2...14.3
.. _89: xolox/python-coloredlogs#89
   ```
   
  
  
   ### 14.2
   ```
   ----------------------------

Honor the ``$NO_COLOR`` environment variable as suggested in issue `88`_.

.. _Release 14.2: xolox/python-coloredlogs@14.1...14.2
.. _88: xolox/python-coloredlogs#88
   ```
   
  
  
   ### 14.1
   ```
   ----------------------------

**Bug fixes:**

- Don&#39;t allow interactive terminal detection to disable colored text when
  colored text is being forced by the caller (reported in issue `84`_).

- Automatically disable colored text when logging output is being redirected to
  a file in such a way that it actually works 😬 (reported in issue `100`_).

**Other changes:**

- Start testing on PyPy 3 (because why not?)

.. _Release 14.1: xolox/python-coloredlogs@14.0...14.1
.. _84: xolox/python-coloredlogs#84
.. _100: xolox/python-coloredlogs#100
   ```
   
  
</details>


 

<details>
  <summary>Links</summary>
  
  - PyPI: https://pypi.org/project/coloredlogs
  - Changelog: https://pyup.io/changelogs/coloredlogs/
  - Docs: https://coloredlogs.readthedocs.io
</details>



Co-authored-by: pyup-bot <github-bot@pyup.io>
Co-authored-by: Ellen Marie Dash <me@duckie.co>
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

No branches or pull requests

5 participants