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

Revert progressreader to not defer close #10327

Merged
merged 1 commit into from
Jan 26, 2015

Conversation

dmcgowan
Copy link
Member

When progress reader closes it overwrites the progress line with the full progress bar, replaces the completed message.

Fixes #10325

When progress reader closes it overwrites the progress line with the full progress bar, replaces the completed message.

Signed-off-by: Derek McGowan <derek@mcgstyle.net> (github: dmcgowan)
@dmp42 dmp42 added this to the 1.5.0 milestone Jan 24, 2015
@dmp42
Copy link
Contributor

dmp42 commented Jan 24, 2015

cc @jfrazelle @icecrime

@crosbymichael
Copy link
Contributor

And what does this break? That change was made to fix an issue right? So reverting this would break something else?

@dmcgowan
Copy link
Member Author

This was change was originally added on the panic fix which converted the progressreader to nopcloser when it passed it into an HTTP request. It was added to be more idiomatic about closing the progressreader. However when the progress reader is closed, it does a write out to the console progress line. Since it is deferred, it overwrites the complete message. To use defer the function scope would need to be decreased, however the close on progressreader does not do anything useful in this case (see other example where progressreader is not closed).

@crosbymichael
Copy link
Contributor

Ok, thanks.

LGTM

@icecrime
Copy link
Contributor

LGTM

1 similar comment
@tiborvass
Copy link
Contributor

LGTM

crosbymichael added a commit that referenced this pull request Jan 26, 2015
Revert progressreader to not defer close
@crosbymichael crosbymichael merged commit db6ecff into moby:master Jan 26, 2015
@dmcgowan dmcgowan deleted the progress-reader-fix branch June 15, 2016 17:14
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.

[V2 Registry] Layers don't show as complete when pushing
5 participants