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 output stream failures (Windows) #4375

Merged

Conversation

memsharded
Copy link
Member

Changelog: Fix: Implemented retrial of output to stdout stream when the OS (Windows) is holding it and producing IOError for output
Docs: Omit

Close #4277

@ghost ghost assigned memsharded Jan 24, 2019
@ghost ghost added the stage: review label Jan 24, 2019
@memsharded memsharded added this to the 1.12 milestone Jan 24, 2019
@lasote lasote changed the title Feature/unzip conan progress bar Fix output stream failures (Windows) Jan 24, 2019
@lasote lasote requested a review from jgsogo January 24, 2019 09:42
Copy link
Contributor

@jgsogo jgsogo left a comment

Choose a reason for hiding this comment

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

Add some disclaimer around the weird for that fixes the issue

if newline:
data = "%s\n" % data

for _ in range(3):
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, this might work and fix the issue, but at least we need some kind of disclaimer for this block of code, maybe a link to the issue, an explanation, (I suppose that a test is impossible)... otherwise anyone having a look at this hack may delete it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jgsogo may be it's possible to mock self._stream.write in test, so it throws an error 3 times?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added comments and a test patching the stream.write.

conans/util/progress_bar.py Show resolved Hide resolved
@memsharded memsharded merged commit 0b7b3ca into conan-io:develop Jan 25, 2019
@ghost ghost removed the stage: review label Jan 25, 2019
@memsharded memsharded deleted the feature/unzip_conan_progress_bar branch January 25, 2019 16:57
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.

4 participants