feat: support dag-scope #767
Conversation
| @@ -0,0 +1,27 @@ | |||
| /** | |||
There was a problem hiding this comment.
I don't like that this is in it's own file.. but i was trying to avoid a circular dep, and it's not quite a "type" so...
There was a problem hiding this comment.
You could make it a type and declare it in the index file?
type DagScope = 'block' | 'entity' | 'all'
Using types instead of enums means we won't need to use --experimental-transform-types in future.
| const isTargetRoot = roots.some(r => r.equals(lastCid)) | ||
| if (!isTargetRoot) { | ||
| throw new Error('knownDagPath must end with one of the target roots') | ||
| } |
There was a problem hiding this comment.
we need some solution here to handle multiple roots where there are paths to each root.
I know this sounds confusing, but "root" according to current kubo/boxo is the "target" root block of a dag.. the "dagRoot" is the root of the dag walked to get to that subdag starting node
|
cspell is yelling about some words in dag-scope.spec.ts |
|
ping @achingbrain @2color I think some other eyes on this would be good, and this is blocking ipfs/service-worker-gateway#356 |
|
|
||
| constructor (components: CarComponents, init: any) { | ||
| this.components = components | ||
| this.log = (init.logger ?? defaultLogger()).forComponent('helia:car') |
There was a problem hiding this comment.
Please use the component logger from Helia, don't create a new one.
| } | ||
| } catch (err) { | ||
| // Handle errors, but don't propagate them to avoid breaking the queue | ||
| this.log.error('Error processing block', err) |
There was a problem hiding this comment.
| this.log.error('Error processing block', err) | |
| this.log.error('error processing block - %e', err) |
| for (let i = 0; i < knownPath.length; i++) { | ||
| const cid = knownPath[i] | ||
| if (!(await blockstore.has(cid))) { | ||
| throw new Error(`CID in knownDagPath at index ${i} not found in blockstore`) |
There was a problem hiding this comment.
Is this correct? What if we want to pull the block from the network?
If the user doesn't want network activity they should be able to pass an offline flag which the networked blockstore will use to throw an error when a block is missing.
| * when the path from dagRoot to the target root is already known, reducing network requests | ||
| * and computation time. | ||
| * | ||
| * Example usage: |
There was a problem hiding this comment.
There's a tag for this:
| * Example usage: | |
| * @example | |
| * |
| * An ordered array of CIDs representing the known path from dagRoot to the target root. | ||
| * The array should start with dagRoot (at index 0) and end with the target root CID. | ||
| * | ||
| * If you provide `knownDagPath`, you should already have verified that the CIDs in the path are present in the blockstore. |
There was a problem hiding this comment.
Does it hurt to let the user control this with an offline flag?
| @@ -0,0 +1,27 @@ | |||
| /** | |||
There was a problem hiding this comment.
You could make it a type and declare it in the index file?
type DagScope = 'block' | 'entity' | 'all'
Using types instead of enums means we won't need to use --experimental-transform-types in future.
| const blockCid = (result as { cid: CID }).cid ?? result | ||
| const newStrategy = (result as { strategy: TraversalStrategy }).strategy ?? strategy |
There was a problem hiding this comment.
Can we avoid the casts here? We don't get type safety like this.
Title
feat: support dag-scope
Description
This PR adds support for the
dag-scopequery parameter to the export and stream methods. This parameter allows the user to specify the scope of the DAG they want to retrieve. The possible values areall,entity, andblock. The default value isall.Also refactored the code quite a bit to support different strategies for walking the dag, and added two new options: dagRoot & knownDagPath.
This code also updates the
blockFiltercheck to occur before theblockstore.getcall to prevent potentially unnecessary network calls.Related ipfs/helia-verified-fetch#198
Related ipfs/service-worker-gateway#356
Notes & open questions
Change checklist