pin: implement pin/ls with only CoreApi#6774
Conversation
019abf2 to
a3588f7
Compare
|
#6705 fixes the issue you found. |
Stebalien
left a comment
There was a problem hiding this comment.
Could you update to your interface-go-ipfs-core dep and rebase on master?
|
@Stebalien as I understand, there is two solutions here:
Can you confirm which one you'd like ? |
|
Let's go with the second. I think we need to evaluate using multiple channels for errors everywhere in the future but that's a larger change. |
a3588f7 to
83bda49
Compare
|
Should be good to go now |
|
Someone to merge this one? |
|
I will look at this when I get a chance (probably not till after the holidays) but I'm going to need to think about the interface changes. |
|
Rebased on master. It would be nice to merge that alongside ipfs/interface-go-ipfs-core#50. |
|
I'll get back to review this after ipfs/interface-go-ipfs-core#50 is through. |
|
@Stebalien do you think this could be included in the 0.5.0 ? That would be neat but it's fine if not. |
|
hey @MichaelMure , this is not making it to 0.5.0 (currently very much in feature-freeze). That said these are on our radar for merging afterwards. |
|
Rebased on master. |
Stebalien
left a comment
There was a problem hiding this comment.
Mostly LGTM but I have one question around cancellation.
core/coreapi/pin.go
Outdated
| if keys.Visit(c) { | ||
| select { | ||
| case ch <- &pinInfo{ | ||
| out <- &pinInfo{ |
There was a problem hiding this comment.
Unless I'm missing something, we still need to select on the context, right?
There was a problem hiding this comment.
Looks like it. Also, ctx should be the first parameter of pinLsAll.
There was a problem hiding this comment.
Should be good now.
This PR use improvements from ipfs/interface-go-ipfs-core#49 and ipfs/interface-go-ipfs-core#50 to implement
pin lsonly using the CoreApi.During this refactor I discovered a bug [I introduced in https://github.com//pull/6493, sorry] where the priority between pin type was not respected: when doing a
ipfs pin ls --type=indirect, direct and recursive pins were not visited first so one of those could have been listed as indirect. This PR also fix this.