Skip to content
This repository has been archived by the owner on Jun 27, 2023. It is now read-only.

Add NodeReifier to fetcher config #13

Merged
merged 3 commits into from
Apr 5, 2021

Conversation

hannahhoward
Copy link
Collaborator

@hannahhoward hannahhoward commented Apr 1, 2021

Goals

Allow configuration of the NodeReifier on Fetcher

Defaults to UnixFS Reifer

Implementation

  • add a few missing function comments
  • produce example demonstrating how it can enable ADLs
  • also adds trusted storage

For Discussion

We could skip the defaulting to the unixfs reifier and push that configuration up — seems unlikely we’ll want to do that though. Then again, ResolveOnce that used to exist in GoPath was the equivalent — and that configuration lived at go-ipfs level

@willscott
Copy link
Contributor

is this still needed with reification merged?

@hannahhoward hannahhoward changed the title Add on demand prototype chooser augmentation Add NodeReifier to fetcher config Apr 2, 2021
@hannahhoward hannahhoward force-pushed the feat/augment-node-builder-chooser branch from 58c7cc8 to d4187fb Compare April 2, 2021 21:54
fetcher.go Outdated
@@ -186,3 +200,5 @@ var DefaultPrototypeChooser = dagpb.AddSupportToChooser(func(lnk ipld.Link, lnkC
}
return basicnode.Prototype.Any, nil
})

var DefaultReifier = unixfsnode.Reify
Copy link
Member

@warpfork warpfork Apr 2, 2021

Choose a reason for hiding this comment

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

Having this as a default in this library seems maybe kinda questionable to me. I don't know exactly where we'd put this library on a graph/layercake/whatever-drawing-format of dependencies and what semantic level we expect of them. But in general I'd support trying to push this as high as possible.

Especially since the default/nil value is functional without being configured.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yea no it doesn't have to go there. I'm mixed about it being there. I think it ties us too much to the IPFS case. Think I'll take it out.

@hannahhoward hannahhoward merged commit 3290471 into main Apr 5, 2021
Jorropo pushed a commit to ipfs/go-libipfs-rapide that referenced this pull request Mar 23, 2023
…lder-chooser

Add NodeReifier to fetcher config

This commit was moved from ipfs/go-fetcher@3290471
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants