Skip to content

Colorlog windows fix#13929

Merged
balloob merged 3 commits intohome-assistant:devfrom
veleek:colorlog-windows-fix
Apr 18, 2018
Merged

Colorlog windows fix#13929
balloob merged 3 commits intohome-assistant:devfrom
veleek:colorlog-windows-fix

Conversation

@veleek
Copy link
Copy Markdown
Contributor

@veleek veleek commented Apr 16, 2018

Description:

Fix colorlog on windows

Modified the way logging is initialized to fix two things.

  1. If the import of colorlog fails the logs will still be formatted
    using the expected HASS log format.
  2. Ensure that logging.basicConfig is called AFTER colorlog is
    imported so that the default handler generated will be writing to the
    wrapped stream generated when colorama is initialized. This allows
    colored logging to work on Windows.

Added support for a --log-no-color command line switch in the event
that someone just wants to disable colored log output entirely.

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#5193

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

If user exposed functionality or configuration variables are added/changed:

veleek added 2 commits April 15, 2018 22:32
Modified the way logging is initialized to fix two things.
1. If the import of `colorlog` fails the logs will still be formatted
   using the expected HASS log format.
2. Ensure that `logging.basicConfig` is called AFTER `colorlog` is
   imported so that the default handler generated will be writing to the
   wrapped stream generated when `colorama` is initialized.  This allows
   colored logging to work on Windows.

Added support for a `--log-no-color` command line switch in the event
that someone just wants to disable colored log output entirely.
Comment thread homeassistant/bootstrap.py Outdated
verbose: bool = False,
log_rotate_days=None,
log_file=None,
log_no_color: bool = True) -> None:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not default to True

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoops, I initially implemented this as log_color, but then switched it around to make using the new switch statement more clean. I'll switch all these to False.

Comment thread homeassistant/bootstrap.py Outdated
log_rotate_days: Any = None,
log_file: Any = None):
log_file: Any = None,
log_no_color: bool = True):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not default to true

Comment thread homeassistant/bootstrap.py Outdated
log_rotate_days: Any = None,
log_file: Any = None):
log_file: Any = None,
log_no_color: bool = True):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not default to true

Comment thread homeassistant/bootstrap.py Outdated
log_rotate_days: Any = None,
log_file: Any = None) \
log_file: Any = None,
log_no_color: bool = True) \
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not default to true

Comment thread homeassistant/bootstrap.py Outdated
log_rotate_days: Any = None,
log_file: Any = None) \
log_file: Any = None,
log_no_color: bool = True) \
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not default to true

@veleek
Copy link
Copy Markdown
Contributor Author

veleek commented Apr 17, 2018

Updated the defaults to false everywhere.

Copy link
Copy Markdown
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! 🐬

@balloob balloob merged commit 7d43ad6 into home-assistant:dev Apr 18, 2018
@balloob balloob mentioned this pull request Apr 27, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Jul 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants