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

Fix stalling in Pipeline command #632

Merged
merged 7 commits into from
Nov 1, 2023

Conversation

astaric
Copy link
Contributor

@astaric astaric commented Jan 18, 2023

When a command (other than the first) in a pipeline wrote more than 64k to the stderr, and the output was consumed with the iter_lines function, the whole pipeline stalled. Fixed by reading the output of all commands where either stdout or stderr was set to PIPE.

This is the same issue as described in
#548

Same problem still exists if command is executed with run(), but that code calls proc.communicate to read stderr/stdout and I do not know how to fix that.

When a command (other than the first) in a pipeline wrote more than
64k to the stderr, and the output was consumed with the iter_lines
function, the whole pipeline stalled. Fixed by reading the output
of all commands where either stdout or stderr was set to PIPE.
somehow stderr includes an empty line when run on pypy
tests/test_pipelines.py Outdated Show resolved Hide resolved
@coveralls
Copy link

coveralls commented Nov 1, 2023

Coverage Status

coverage: 84.548%. first build when pulling 3acc051 on astaric:pipeline-stalling into d3ea8e3 on tomerfiliba:master.

@henryiii henryiii merged commit 38a108b into tomerfiliba:master Nov 1, 2023
31 checks passed
@vient
Copy link
Contributor

vient commented Nov 1, 2023

@henryiii do you plan to release new version soon? Stalling pipelines is something we've bumped in several times, this fix seems important.

I don't know how new releases are made, but if there is anything I can help with, I am ready :)

@henryiii
Copy link
Collaborator

henryiii commented Nov 1, 2023

Yeah, I'm planning one pretty soon. I want to get a release out then drop Python 3.6.

@desertfury
Copy link

Hi @henryiii !
I wonder have you shipped release that contains this particular fix yet? Thanks!

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.

5 participants