-
Notifications
You must be signed in to change notification settings - Fork 15
Conversation
hugomrdias
commented
May 20, 2020
•
edited
Loading
edited
- fix remote node ipfs http client path
- fix rabin params go-ipfs has validation and returns errors
- fix use cid instead of hash in the pin tests
@@ -354,7 +354,7 @@ describe('files', function () { | |||
|
|||
it('rabin chunker small chunks', () => { | |||
const options = { | |||
chunker: 'rabin-16-16-16', | |||
chunker: 'rabin-16-32-64', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this change necessary? rabin-16-16-16
seems to be supported by js and go:
$ ipfs add --chunker=rabin-16-16-16 ./package.json
added QmTUSudDKFYaVHLdU1wRoHYXkydQaDch4Kt8CwuVFShpiU package.json
$ jsipfs add --chunker=rabin-16-16-16 ./package.json
added QmTUSudDKFYaVHLdU1wRoHYXkydQaDch4Kt8CwuVFShpiU package.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
api returns validation errors, min needs to bigger than avg, avg need to be smaller than max etc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which api? The js-IPFS api? Should probably fix that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't confirm, but I'm assuming go http api we didn't change anything.
I will look into it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, no - I think you're correct, it's go-IPFS that's changed, I think I was running an old version just now (still 0.5.x but an old version from go-ipfs master):
$ ipfs add --chunker=rabin-16-16-16 ./package.json
Error: incorrect format: rabin-min must be smaller than rabin-avg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Stebalien was this change intentional?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This chunker config is equivalent to a fixed-sized chunker. I'm not sure if this was intentional, but I'd like to leave it as it helps detect misuse of the chunker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added this item here ipfs/js-ipfs#3030 so we replicate in js-ipfs