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

Logging: explicitly set encoding for file handler #3327

Merged
merged 3 commits into from
Sep 2, 2021

Conversation

jenshnielsen
Copy link
Collaborator

This prevents issues where the broken pipe character (used for delimiting messages) cannot be written to the log file if the file has a different encoding. Reading the documentation it is not clear to which extend this may also be an issue for the stream handler used for the console logging. This writes to stderr. According to https://docs.python.org/3/library/sys.html#sys.stderr the encoding of stderr should be defined by local on oses other than windows so this probably works without issues on windows (but there are some limitations as noted in the stderr documentation) On most other platforms I would expect local encoding to be utf8 anyway.

@codecov
Copy link

codecov bot commented Sep 1, 2021

Codecov Report

Merging #3327 (f3313f6) into master (ed825e6) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3327   +/-   ##
=======================================
  Coverage   66.20%   66.20%           
=======================================
  Files         220      220           
  Lines       29250    29250           
=======================================
  Hits        19365    19365           
  Misses       9885     9885           

@jenshnielsen jenshnielsen changed the title explicitly set encoding for file handler Logging: explicitly set encoding for file handler Sep 1, 2021
@FarBo
Copy link
Contributor

FarBo commented Sep 1, 2021

This seems the way to go to fix the broken pipe encoding.
In a quick search, older Linux server encoding is something else. Newer versions are adapted to utf-8. The rest should be utf-8.

@jenshnielsen
Copy link
Collaborator Author

Looks like this fixed the reported issue. @astafan8 could you review

@jenshnielsen jenshnielsen merged commit dd1e45c into microsoft:master Sep 2, 2021
@jenshnielsen jenshnielsen deleted the always_write_utf8_files branch September 2, 2021 12:03
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

Successfully merging this pull request may close these issues.

None yet

3 participants