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

did cat lose the progress bar? #1795

Closed
jbenet opened this issue Oct 4, 2015 · 17 comments
Closed

did cat lose the progress bar? #1795

jbenet opened this issue Oct 4, 2015 · 17 comments
Assignees

Comments

@jbenet
Copy link
Member

jbenet commented Oct 4, 2015

I'm not seeing it

> ipfs cat QmXipyrjt2Vzjg2vV7uhgCi8GNEFhrEdXrirtDcYhqGGEc >r3

cc @rht

@jbenet
Copy link
Member Author

jbenet commented Oct 4, 2015

QmXipyrjt2Vzjg2vV7uhgCi8GNEFhrEdXrirtDcYhqGGEc is ~200MB of randomness.

@rht
Copy link
Contributor

rht commented Oct 4, 2015

The err happens only when daemon is on
Error: read tcp 127.0.0.1:53124->127.0.0.1:5001: read: connection reset by peer.
When offline, sometimes it shows progressbar when it is slow enough.

@jbenet
Copy link
Member Author

jbenet commented Oct 4, 2015

isn't that error different? the cat actually completes

@jbenet
Copy link
Member Author

jbenet commented Oct 4, 2015

And, are you getting cat progressbar at all on daemon now? it used to be there-- do we need to bisect this?

@rht
Copy link
Contributor

rht commented Oct 4, 2015

No, the cat err when daemon that it doesn't complete. Bisecting.

@rht
Copy link
Contributor

rht commented Oct 4, 2015

Found a bug: coreunix.Cat should have ctx as arg.

@rht
Copy link
Contributor

rht commented Oct 4, 2015

At least for the connection reset by peer err:

a7e50f1 is the first bad commit

commit a7e50f1fbc5a58963a1b1e462575249de61540f1
Author: Jeromy <jeromyj@gmail.com>
Date:   Fri Jul 24 17:41:59 2015 -0700

    implement http trailers for errors after headers are sent

    refactor http handler and copyChunks to get this all to work correctly
    License: MIT
    Signed-off-by: Jeromy <jeromyj@gmail.com>

@rht
Copy link
Contributor

rht commented Oct 4, 2015

a7e50f1

@rht rht self-assigned this Oct 4, 2015
@whyrusleeping
Copy link
Member

yeah, the connection reset by beer is a result of us not properly handling trailers, and will be fixed by #1621 when we make go1.5 a requirement.

@rht
Copy link
Contributor

rht commented Oct 4, 2015

What would be the fix in the mean time? Offline cat?

@rht
Copy link
Contributor

rht commented Nov 5, 2015

Can be closed?

@whyrusleeping
Copy link
Member

it doesnt look like cat has a progress bar still... thats odd

@rht
Copy link
Contributor

rht commented Nov 5, 2015

Bisecting, 814f437 is the latest good commit. The first bad commit is still the same as the pointed out in #1795 (comment).

@rht
Copy link
Contributor

rht commented Nov 7, 2015

One likely cause: a7e50f1#diff-ceb40d01c16a02936d808eb855ea499bR293 wraps outside of the progressbar reader.
Would it be clean to use channel (just as the one used in ipfs add)?

@jbenet
Copy link
Member Author

jbenet commented Nov 10, 2015

fwiw, im not seeing the progressbar on add either. what happened?

cc @whyrusleeping

@jbenet
Copy link
Member Author

jbenet commented Nov 10, 2015

and we should test that progress bar works so that this doesnt happen again

@daviddias
Copy link
Member

Following @whyrusleeping's comment #736 (comment), we now have a progressbar, sometimes it takes time to appear when the dir is too large. Closing this one in favor of keeping progressbar enhancements discussion on #736

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

No branches or pull requests

4 participants