chore: update ipfs-http-client to the latest version #23
Conversation
|
Depends on ipfs/js-ipfs#2965 |
| searchParams: { | ||
| timeout: `${options.timeout}ms`// The api requires specification of the time unit (s/ms) | ||
| } |
There was a problem hiding this comment.
| searchParams: { | |
| timeout: `${options.timeout}ms`// The api requires specification of the time unit (s/ms) | |
| } | |
| timeout: options.timeout |
i was looking into this in vasco's PR and the string timeout doesn't work correctly for some reason.
There was a problem hiding this comment.
Hmm, are you sure? Against go-ipfs:
$ time curl http://localhost:5001/api/v0/dag/get?arg=/ipfs/QmaA9foTB68jrGgP3dBcyc8nrjbfAji8xrJsrJYo22BiVo\&timeout=5s
{"Message":"failed to get block for QmaA9foTB68jrGgP3dBcyc8nrjbfAji8xrJsrJYo22BiVo: context deadline exceeded","Code":0,"Type":"error"}
real 0m5.044s
user 0m0.006s
sys 0m0.015
$ time curl http://localhost:5001/api/v0/dag/get?arg=/ipfs/QmaA9foTB68jrGgP3dBcyc8nrjbfAji8xrJsrJYo22BiVo\&timeout=5000
real 0m0.057s
user 0m0.007s
sys 0m0.014s
$ time curl http://localhost:5001/api/v0/dag/get?arg=/ipfs/QmaA9foTB68jrGgP3dBcyc8nrjbfAji8xrJsrJYo22BiVo\&timeout=1
real 0m0.048s
user 0m0.007s
sys 0m0.017s
$ ipfs dag get /ipfs/QmaA9foTB68jrGgP3dBcyc8nrjbfAji8xrJsrJYo22BiVo --timeout 1
Error: time: missing unit in duration 1There was a problem hiding this comment.
i was talking about the client timeout, with string the tests timeout instantly with number they run.
There was a problem hiding this comment.
Context: #25
I did not re-try this yet with the fix hugo's recommended
There was a problem hiding this comment.
I think when these tests were written, we only had timeouts in a few HTTP method calls - the argument here is for the remote node, I think.
We could do both and set the timeout in the client and the server, but that could make it non-deterministic, unless we don't inspect which timeout was the one that timed out..
There was a problem hiding this comment.
Ideally we should perform set the timeout for the client and allow the cancel of that to cause the remote context to close. It sounds like there are some downstream issues with this at the moment though, so for now we can use the remote timeout to get around this until context cancelling is working appropriately.
We could do both and set the timeout in the client and the server
Definitely want to avoid that, it would be a nightmare to deal with.
No description provided.