pinning/pinner: Pinner should not be tasked to fetch blocks
- Dominant language
- Go
- Stars
- 316
- Forks
- 163
- Avg merge
- 4d 12h
- Merged PRs (30d)
- 8
Description
Having worked on the Pinner in various way (including alternate implementation), it occurred to me that it really shouldn't be tasked to:
1. write the root block into storage in `Pin()`: https://github.com/ipfs/go-ipfs-pinner/blob/72f5e02e73db5e05ef0a140a7938cbc89dfc38b0/dspinner/pin.go#L177
2. fetch the full DAG in `Pin()`: https://github.com/ipfs/go-ipfs-pinner/blob/72f5e02e73db5e05ef0a140a7938cbc89dfc38b0/dspinner/pin.go#L202
This is for different (not obvious, I admit) reasons:
- often, the caller side know if the blocks are local or not. In any case, it's trivial to fetch the blocks on the caller side if not.
- that would allow to merge `Pin` and `PinWithMode` and simplify the API
- this hurts composability in weird scenario like mine, where you want to control where that fetching is happening, and where the blocks are going (aka, not necessarily in the normal DAGService)
- it might avoid double writes
- it seems like this pain is already felt within kubo: https://github.com/ipfs/kubo/blob/82467bc936fe24355c930f7226fce7e41fee591c/core/commands/dag/import.go#L73-L87
Admittedly, things have worked that way for a long time, but fixing that might help in the long run. Looking at where `Pin` is called in kubo, it also looks like a non-invasive change.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.