-
Notifications
You must be signed in to change notification settings - Fork 198
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
Broken Connection on successive calls using add_bytes #187
Comments
For completeness, before executing the above Python, I started the IPFS daemon locally:
I also updated to Python 3.7.3 using homebrew and experience the same issue. |
This looks to have been introduced between ipfsapi 0.4.3 and 0.4.4. With code ( import ipfsapi
import os
bytes1 = bytearray(os.urandom(100000))
IPFS_API = ipfsapi.Client('localhost', 5001)
res1 = IPFS_API.add_bytes(bytes1)
res2 = IPFS_API.add_bytes(bytes1)
print(res1)
print(res2)
...then updating to ipfsapi 0.4.4 with a fresh virtualenv (which seems to also install
|
Thanks you for this detailed analysis of the problem, unfortunately ipfsapi v0.4.3 and ipfsapi v0.4.4 (which is now a wrapper around ipfshttpclient) are really two different libraries. The versioning is somewhat deceiving here unfortunately, ipfshttpclient actually started with version 0.4.11 which is a lot closer to the truth. If you have the time and knowledge, could you do a |
Sorry, I just noticed again that your originally did use |
@Alexander255 Thanks for the acknowledgement. I encountered this issue in attempting to update https://github.com/oduwsdl/ipwb to the latest, refactored, renamed I am not familiar with |
Using |
1b5b1c0 9edc945 |
Another finding, the headers in 9edc945 contain "Connection: close" when in 1b5b1c0 they do not: 9edc945 1b5b1c0 |
...seems to be the source of the header mentioned above. ipfs/kubo#5168 has since been closed. Removing this explicit Connection header specification fixes the problem on my initial testing. EDIT: the issue remains. The pipe still breaks even when the headers do not contain
|
An optimistic update: I updated my go-ipfs to 0.4.21 and the original The README states that 0.4.20 is blacklisted but was the issue that caused it to be so resolved in 0.4.21, @Alexander255? If so, we can proceed with migrating to ipfshttpclient with the insistence of a go-ipfs installation from ipwb users. /cc @ibnesayeed |
Thanks @machawk1, if the new version of this library works well with the new version of the go-ipfs then we can move on. |
@machawk1: Thanks for your tremendously valuable research here! While, as @ibnesayeed found out, adding the header should not be a problem anymore, it's still an unnecessary performance penalty and I'm glad to here that this can be removed with go-IPFS v0.4.19+. I will create a patch that removes the The problem with 0.4.20 was that several API methods had returned broken or incomplete responses making it basically useless if you relied on any of these. Hence I decided to blacklist it altogether, rather than dealing with support queries that I can do nothing about anyways. (As such I don't think it was related to this, but maybe played a part as well.) |
@Alexander255 What sort of testing would be required to ensure this module's full functionality with go-ipfs v0.4.21+? Do the issues that prevent the recommendation of v0.4.20 also exist in v0.4.21? Given |
Damn! Looks like I forgot the upload the changes I'm planning to land, somehow I thought them to already being released. @machawk1: Please see and comment on #189. Thank you! The issues in v0.4.20 were of a different nature, iirc the daemon sent incomplete responses to several calls. I can look it up if you want to know more. |
Support for 0.4.19 and below was dropped, so this isn't an issue anymore. |
I am attempting to debug an issue wherein I am pushing the binary image data represented in Python byte string into IPFS using ipfshttpclient.
I have been able to distill this down to an example with two successive calls of
add_bytes
with large byte strings:...produces:
Changing the above line to
bytes = bytearray(os.urandom(10))
, using10
instead of100000
does not produce this exception.The text was updated successfully, but these errors were encountered: