Skip to content

Conversation

@achingbrain
Copy link
Member

Reuse the existing exporter walkPath method to traverse a DAG as it is clever enough to not load unnecessary blocks which is important when traversing HAMTs.

Change checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation if necessary (this includes comments as well)
  • I have added tests that prove my fix is effective or that my feature works

Reuse the existing exporter `walkPath` method to traverse a DAG as
it is clever enough to not load unecessary blocks which is important
when traversing HAMTs.
@achingbrain achingbrain requested a review from a team as a code owner February 27, 2024 09:04
Copy link
Member

@SgtPooki SgtPooki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm. I knew it should have been easier than Adin and I were making it out to be

Comment on lines +125 to +128
// should be able to cat the index file without loading the other files
// in the shard - the blockstore is offline so will throw if requested
// blocks are not present
const bytes = await toBuffer(fs.cat(dir.cid, {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Nice 👍

Comment on lines +41 to 46
const p = `/ipfs/${cid}${path == null ? '' : `/${path}`}`
const segments = await all(walkPath(p, blockstore, options))

if (segments.length === 0) {
throw new DoesNotExistError('Could not find path in directory')
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is so much cleaner.

@achingbrain achingbrain merged commit 6f8c15b into main Feb 27, 2024
@achingbrain achingbrain deleted the fix/use-unixfs-exporter-to-traverse-hamt branch February 27, 2024 16:48
@achingbrain achingbrain mentioned this pull request Feb 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants