Pinning support #11
Comments
|
Would there be guidance available for this? I'd be interested in tackling it |
|
I guess the best is to dive in. What sort of guidance did you have in mind? You probably want to have a table in the datastore If you have an ipld datastructure you will want to pin the root. After you make a change you want to pin the new root and unpin the old one. We might end up requiring two different types of pinning, as when streaming for example we only the buffered part of the file to be pinned. There we want to manage individual blocks manually. |
|
Related: the discussion in #84 and the ipfs/specs discussion linked from there. |
|
I didn't have anything too specific in mind, just wanted to check that you guys would be willing to support the changes since I'm still ramping up on IPFS stuff! I'll take a look and hopefully have a PR up soon |
|
Great @saresend and welcome! We look forward to it. |
|
Have something very tentative up at #117, would be interested in seeing if the direction here is correct! |
|
@dvc94ch Should we leave this open to capture the remaining work or close and create new issue(s)? |
|
For posterity: The testing of #90 requires |
|
Hey, sorry that I haven't got around to it just yet, expect a PR up in the next couple of days! @koivunej Let me know if you want to take that work, don't want to duplicate if you're already working on it. |
|
@saresend I'll be soon (hopefully!) moving over to this. I want to handle #238 first though and lets just say it hasn't been so fast moving as I would had hoped. After #238 moving Regarding #152. If you want, we could iron out what exists at the moment and get it merged in. Let me know. |
332: refactor: ipfs.dag().resolve(..) from ipfs-http r=koivunej a=koivunej This PR continues refactoring/rewriting the ipfs-http IpfsPath resolving, so that we can move refs into ipfs, so that we can implement pinning and unpinning. Realized while doing this that perhaps pinning api operated just on Cids so this may have been unnecessary, however refs works on IpfsPaths so not really sure yet. The result of `IpfsDag::resolve` may be already loaded block, it's actually always a loaded block. Made some changes to ipfs::unixfs::cat to accept either `Block` or `Cid`. There may have been similar changes as well. Step forward on #238 and towards #11. It is more lines added than removed probably due to more refined return types and solved local resolving or not following links (the test doesn't work though but the concept or implementation hint makes sense). Previous version did not support local resolving at all, nor could that had been added there in any way. Co-authored-by: Joonas Koivunen <joonas@equilibrium.co>
337: refactor: move refs as Ipfs::refs and ipfs::refs::iplds_refs r=koivunej a=koivunej This PR moves `refs` from `ipfs-http` to `ipfs`. Ended up moving only a `iplds_refs` which needs pre-resolved `IpfsPaths`. As for short term I think this should help towards #11. I was originally planning to make this into a version which would accept both `IpfsPath` (or `CidPath`) and already projected `(Cid, Ipld)` but the duplication of code would be quite unavoidable, as the http side will have to "buffer" all IpfsPath resolution *before* calling a version of `iplds_refs` which would accept a `TryStream<Ok = (Cid, Ipld), Error = _>` instead of `IntoIterator<Item = (Cid, Ipld)>`. I did continue this solution and did try out prefetching options for unixfs_cat for example, but I'll leave them for next PRs since they will complicate the API quite a bit. While moving the tests, I removed of the "duplicate" dependency on `hex` as `hex-literal` is a bit more straight forward. ### Checklist (can be deleted from PR description once items are checked) - [x] **New** code is “linted” i.e. code formatting via rustfmt and language idioms via clippy - [x] There are no extraneous changes like formatting, line reordering, etc. Keep the patch sizes small! - [x] There are functional and/or unit tests written, and they are passing - [x] There is suitable documentation. In our case, this means: - [x] Each command has a usage example and API specification - [x] Rustdoc tests are passing on all code-level comments - [x] Differences between Rust’s IPFS implementation and the Go or JS implementations are explained Co-authored-by: Joonas Koivunen <joonas@equilibrium.co> Co-authored-by: Joonas Koivunen <joonas.koivunen@gmail.com>
342: feat: http pin/add r=ljedrz a=koivunej Continued from #152 by @saresend cc #11 . Closes #152. This only adds the `pin/add` per current impl which passes the tests which don't verify anything. I am inclined to merge this in as is since the next PR is likely going to be a larger one. ### Checklist (can be deleted from PR description once items are checked) - [x] **New** code is “linted” i.e. code formatting via rustfmt and language idioms via clippy - [x] There are no extraneous changes like formatting, line reordering, etc. Keep the patch sizes small! - [x] There are functional and/or unit tests written, and they are passing - [ ] There is suitable documentation. In our case, this means: - [ ] Each command has a usage example and API specification - [ ] Rustdoc tests are passing on all code-level comments - [ ] Differences between Rust’s IPFS implementation and the Go or JS implementations are explained Co-authored-by: saresend <saresend@usc.edu> Co-authored-by: Joonas Koivunen <joonas@equilibrium.co>
|
Ah, so sorry just saw this. I've started work and been very busy. I should have some time later this week however, if there hasn't been too much development on this front since! |
|
Awesome, sounds good! Sorry I went mia been moving for work and getting settled and haven't had too much time for open source lately. If there's still opportunity for me to help out on this feature, I'd love to re-involve myself! |
|
Regarding the topic of pinning, my #349 is not going to close the chapter at all. The #349 is "ok" for inmemory version. Once that's done, I'll continue over to a persistent block and data store with a bit different design: instead of "writing" all indirect pins explicitly I'll probably settle for a document per recursive pin, as bigger files are not a huge issue but gazillion small files are. Also, crash safety is easier to work towards with more atomic writes. I might still change the plans if I run into huge issues since we are at the moment on a deadline. Regardless of how it ends up looking like I am already certain the solution will not perfect and help wanted/enhancement issues should be appearing in the near future :) |
|
Sounds good, I'm not sure I'm the right person to chime in on architecture planning, but would absolutely be interested to follow along! |
|
Either way, here's the list of issues that are marked as "help wanted" or "good first issue", the latest of which was just added :) |
|
Great to hear that @saresend! My latest batch of big PRs ending in #358 and #360 are an initial, feature-full as far as I know for fs based pinning. As you follow along be welcome to join in any reviews or you can open an issue. The more understandable the code is the better it is for everyone. Closing as following the PRs #342, #349, #355, #358, #360 are merged. Known unfinished parts are tracked in at least #350, #351, #352. |
Related issue is garbage collection of unpinned blocks
The text was updated successfully, but these errors were encountered: