Conversation
lib/download.js
Outdated
| super() | ||
|
|
||
| // it's a batch of downloads | ||
| if (Array.isArray(drive)) { |
There was a problem hiding this comment.
This works when combined with the if in _open, but it feels like a hack. It's not the fault of your PR, as I think you solved it in the simplest possible way given that downloadDiff creates a Download object with a completely different signature than expected.
Needs @mafintosh 's input before making any changes, but IMO we need an additional object which just manages a list of downloads and has a method to indicate when they're all done. Then downloadDiff can just return that directly, instead of passing through this object.
|
|
||
| test.skip('drive.downloadRange(dbRanges, blobRanges)', async (t) => { | ||
| const { drive, swarm, mirror, corestore } = await testenv(t) | ||
| test('drive.downloadRange(dbRanges, blobRanges)', async (t) => { |
There was a problem hiding this comment.
The previous test was testing something else entirely it seems. Your test makes more sense to me, but needs a good look from @mafintosh too because we're changing both the tests and the main logic, which is always dangerous.
Likewise for the other test
index.js
Outdated
| for await (const entry of this.diff(length, folder, opts)) { | ||
| if (!entry.left) continue | ||
| const b = entry.left.value.blob | ||
| if (!entry.right) continue |
There was a problem hiding this comment.
I don't think your fix is correct, at least per what the README says, so you might be paving over another bug.
In drive.diff(version, folder), left is the entry in the current version, and right is the entry in the previous version (drive at length).
So logically, we want to download all blobs that are present in the current version (since those changed, as per the this.diff contract). This means we want to download all entries who have left set, which is what the previous code was doing.
There was a problem hiding this comment.
yeh reverted this after convo with rafa
rafapaezbas
force-pushed
the
fix-download-range-and-diff
branch
from
1eb9dde to
0006f8a
Compare
3 participants
No description provided.