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

Reconfigure stdout to be line-buffered in quickrun #468

Merged
merged 1 commit into from
Jun 26, 2023

Conversation

dwhswenson
Copy link
Member

@dwhswenson dwhswenson commented Jun 21, 2023

It looks like we're having an issue that, if a job is killed by a queueing system, some of the buffered output might not get written to the job's output file. This should force the buffer to flush on newline, which can hopefully solve some user confusion.

I considered putting this in the CLI main, but decided the one-liner can go in specific commands that need it. It should only be needed for commands that will be long-running (on a cluster).

@codecov
Copy link

codecov bot commented Jun 21, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (9e6766e) 91.99% compared to head (62b1a83) 92.00%.

❗ Current head 62b1a83 differs from pull request most recent head 9e39664. Consider uploading reports for the commit 9e39664 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #468   +/-   ##
=======================================
  Coverage   91.99%   92.00%           
=======================================
  Files         106      106           
  Lines        6324     6326    +2     
=======================================
+ Hits         5818     5820    +2     
  Misses        506      506           
Impacted Files Coverage Δ
openfecli/commands/quickrun.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@dwhswenson
Copy link
Member Author

@mikemhenry : Could you check if this does, in fact, solve the problem? I was thinking you could use something like the shorten_runs.py script to make units shorter, then set some even-shorter wall time. Rather than running this code, you could also make a test run like that and compare with PYTHONUNBUFFERED unset and set: ideally, you can regularly reproduce when unset, and never reproduce when it is set!

@mikemhenry
Copy link
Contributor

Reference here: https://docs.python.org/3/library/io.html#io.TextIOWrapper (I was trying to understand how setting something abut buffering to True would reduce buffering, but it flushes the buffer on \n)

@mikemhenry
Copy link
Contributor

RE: testing

I will have to make a script that does something like sleep and print in a loop because it looks like dumping all the debug stuff has made everything show me the progress as I look at it

@richardjgowers richardjgowers enabled auto-merge (rebase) June 26, 2023 10:16
@richardjgowers richardjgowers merged commit a8c0c92 into main Jun 26, 2023
7 checks passed
@richardjgowers richardjgowers deleted the quickrun-unbuffered branch June 26, 2023 10:36
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