Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Allow verbose/quiet level to be specified via config file and env var #8578
Allow verbose/quiet level to be specified via config file and env var #8578
Changes from all commits
2439d80
7a0061d
a85be3f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I spent a solid minute trying to figure out why I wrote it as I did it 😄 Turns out it was to ensure that the file is written to disk (which is warrantied by
f.__exit__
). I think we can usef.flush()
as well although I'm not sure enough about NamedTemporaryFile semantic to tell if that is safe on all platforms.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The file is definitely written on every newline AFAICT, so I think @xavfernandez's suggestion will work well for simplifying this logic. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI writing newline doesn't write to disk (at least not by default), but thanks for raising this up. The best source I can refer to is ISO C99 7.19.3.3 which states:
I dig down CPython docs again and found (NamedTemporaryFile passes
buffering
to open):So if we set
buffering=1
the file would beflush
ed upon newline, otherwise after a few KiB. However,TextIOWrapper.flush
does not seem warranty to flush the filesystem file (or maybe I interpret the comment incorrectly, please don't ask me what the code actually does).That being said, I'm pretty sure on most platforms it is safe to assume
flush
would do the job and would not object someone else applying the patch above. Personally I just do know enough to be responsible for such change.