feat: add timeout defaults #6
Conversation
License: MIT Signed-off-by: Jacob Heun <jacobheun@gmail.com>
ghost
assigned 
src/index.js
Outdated
| * | ||
| * @param {CID} key | ||
| * @param {number} _timeout This is ignored and is only present to comply with the dht interface | ||
| * @param {number} timeout How long the query can take. Defaults to 30 seconds |
There was a problem hiding this comment.
Since we are changing this, maybe we should change this param to an options object with the timeout property, and for now, keep accepting it as a number, but deprecate in the future, in favor of options.
There was a problem hiding this comment.
The major reason I didn't make the change to options was to comply with how the dht is doing things, https://github.com/libp2p/js-libp2p-kad-dht/blob/0a2f9fe418d364356d27152be6a7279ebb220d2c/src/index.js#L450. I'm good making the change but we should do that in the dht as well.
There was a problem hiding this comment.
You are right! I am just concerned that we may need to change all them in the future. I Can create a PR for kad-dht adding that behavior, what do you think?
License: MIT Signed-off-by: Jacob Heun <jacobheun@gmail.com>
|
Updated the timeout change to match the kad dht changes libp2p/js-libp2p-kad-dht#45 |
The PR aims to mitigate an issue where connections to the delegate node would stay open for an extended period. This change will pass the
timeoutparam to the API allowing for better limiting.findProviders, which is compatible with the kad dht. This also sets a default limit of 30 seconds to limit the http calls to the delegate.Reference: ipfs/kubo#4595 (comment)
I have verified this using the WIP libp2p implementation and a local go-ipfs delegate node.