Update for LinkSystem #161
Conversation
DO NOT MERGE
replace most instances of loader and storer with ipld.LinkSystem
|
@warpfork would love to get your opinion on this. |
hannahhoward
changed the title
| if tb.LinkSystem.DecoderChooser == nil { | ||
| t.linkSystem.DecoderChooser = defaultLinkSystem.DecoderChooser | ||
| } | ||
| if tb.LinkSystem.EncoderChooser == nil { | ||
| t.linkSystem.EncoderChooser = defaultLinkSystem.EncoderChooser | ||
| } | ||
| if tb.LinkSystem.HasherChooser == nil { | ||
| t.linkSystem.HasherChooser = defaultLinkSystem.HasherChooser | ||
| } |
There was a problem hiding this comment.
I think these might be kind of excessive? LinkSystem should give reasonable error messages if these are unset, so I think it's reasonable to presume their existence. Not at all a strongly held opinion though; if this made an API easier to use, carry on.
|
Agree, the big design question is if things should mostly pass around LinkSystem or mostly pass around BlockWriteOpener+BlockReadOpener and construct a LinkSystem on demand. These diffs went for the former. The result looks totally reasonable. 👍 |
| // from an IPFS blockstore | ||
| func LoaderForBlockstore(bs bstore.Blockstore) ipld.Loader { | ||
| return func(lnk ipld.Link, lnkCtx ipld.LinkContext) (io.Reader, error) { | ||
| // LinkSystemForBlockstore constructs an IPLD LinkSystem for a blockstore |
There was a problem hiding this comment.
is this repo (graphsync) the best place for this to live? can we put it in the blockstore and/or in cidlink?
(@warpfork do you think this would be reasonable to live in cidlink?)
There was a problem hiding this comment.
That's a good question. I don't have a strong opinion.
If we end up writing this a lot of times in a lot of places, then maybe it makes sense to put it somewhere in the go-ipld-prime repo. Probably not the cidlink package directly -- because I'd like use of that package to drag in go-cid and nothing else -- but some other glue package of the same general pattern as cidlink could end up being reasonable.
Maybe putting it in the blockstore repo could make sense, too. I think that would tend to work and should not result in cycles.
There was a problem hiding this comment.
it should migrate IMHO, eventually.
hannahhoward
changed the title
hannahhoward
force-pushed
the
linksystem
branch
from
20d41d6
to
265fd4a
Compare
hannahhoward
force-pushed
the
linksystem
branch
from
265fd4a
to
0fd517c
Compare
Goals
Update for IPLD's LinkSystem
Implementation
There were a couple ways to go here, I went for the one that's more breaking, but more in link with the linksystem design.
Basically, anywhere we take in a loader and a storer, now we take a linksystem. I don't know if we will encounter loads of places where we might replace the encoders, decoders, and hashers, but it seems worth doing.
The alternative would just be to replace the old ipld.Loader & ipld.Storer with ipld.BlockReadOpener and ipld.BlockWriterOpener
I kept it this way for a few low level calls.