-
-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pinning semantics slightly confusing. #590
Comments
my thoughts, are that
|
Agreed on the "but not indirect", but consider this user-misleading case: # suppose h1 points to h2
# i mean to pin h2 and its siblings, which are in h1.
ipfs pin add -r <h1> # pins h2 indirectly
# some time later, i want h1 itself.
ipfs pin add <h1> # i forgot i already had pinned h1 recursively
ipfs pin rm <h1> # oh nvm i dont want it anymore
# h2 now unpinned silently. oops. What about something like this: # if h1 is already pinned recursively:
> ipfs pin add <h1>
# idempotent no-op, h1 is already pinned.
> ipfs pin rm <h1> # oh nvm i dont want it anymore
error: <h1> is pinned recursively, to remove recursive pins, use:
ipfs pin rm -r <h1> So -- unless im not considering something which is highly probable -- maybe effectively we can say that the user must use
Great, i like this. (and looking at the original description we discussed it then too: #172 ) |
Okay, so if h1 is pinned both direct and recursive, |
im not certain this is good UX yet, but it addresses the confusion you outlined above. We've two pick between to models:
it may be worth evaluating whether |
I think im leaning towards recursive being the default |
it's probably what is meant most often. we shouldnt remove the |
okay, do you want this on its own PR? ive been digging into it on my gc command branch since i need it for proper testing |
ideally own pr, for independent testing |
what i often do is rebase those commits to the beginning of my branch, spin them out as a separate branch and keep working (see bootstrap-fix in relation to addr-explosion) |
The more i tinker with this, the more i think that you shouldnt be able to have something both Directly and Recursively pinned. Example:
|
This leads to the case described here: #590 (comment) |
What about this use case:
It should remove the direct pin on h2 but leave the indirect pin on h2 (because h1 is pinned recursively). h2 should stay around because of h1. Now this one:
h2 should stay around because it was pinned recursively, even if the indirect pin inherited from h1 was removed. h3 should also stay around because its indirect pin inherited from h1 is removed, but its indirect pin inherited from h2 is still there. |
It might be worth (eventually) having a flag for rm that allows demotion from a recursive pin to a direct pin. |
it doesn't help that |
@anarcat pinning:
It would be really helpful to get someone outside the core dev team to help us with the help docs. we need to strike a good balance between our biased way of thinking and how a newcomer would approach the concepts. who can help us with this? |
i threw a bunch of issues in - how can i help? :) i this case, wouldn't a change in the help suffice? |
@anarcat yeah in this case a simple change to the help would... help. check it out here: https://github.com/jbenet/go-ipfs/blob/master/core/commands/pin.go the commands are pretty ugly go code, but it has all the help text in one place |
re:
@jbenet i wonder if the current approach of having "everything in |
I want to have:
It's here: https://github.com/protocol/websites/tree/master/ipn.io/src/ipfs.io
Yeah, i've been reluctant because i cant lead this effort atm, but happy to guide |
this issue tripped me up...i'll look into the codebase over the next week. Noob perspective should help too |
@zramsay yes please! |
So it took me awhile to pin a file successfully - because I didn't realize that For the API, if there's an error, the unmarshalled response.Body fits nicely in the Error struct. Otherwise, it's decoded into a []byte. It works fine, but leads to messy code, especially w/ an abstraction. e.g. I have a func PostAPICallToIPFS() that parses & deals with errors, but has to return the (success) response to the func that called it then parse it. It'd be nice to have a standard struct for all http responses; whether they are succesful or error. |
Curious if you saw the pinning example in http://ipfs.io/docs/examples ?
The expectation at the moment is for the common case of a user who knows almost nothing about how ipfs works under the hood-- that if "user adds X to ipfs" then "user expects X to be in ipfs later". Maybe a
Yeah... do you have a concrete code example we can look at together to fix the right UX? |
Yeah when I finally came across it, everything made sense. Probably should've also spent more time playing with the cli beforehand.
Gotcha.
That's a perfect task for me to get more familiar with the codebase.
Yup |
great! 👍 (cc @whyrusleeping because this will need the lock stuff un the 0.4.0 branch) |
nothing should change with respect to the locking stuff. we already lock around the add command, extra flags shouldnt change anything |
@whyrusleeping it may be added to the |
btw, @zramsay have you seen https://github.com/whyrusleeping/ipfs-shell ? we're creating an API wrapper slowly. see it used here: https://github.com/whyrusleeping/pinbot |
@whyrusleeping (and @wking) may be time to hammer our ipfs-shell. shall we do it in that repo? @whyrusleeping want to move |
that wrapper is exactly what we need! |
i could move it to |
@zramsay if you need any help with the ipfs-shell package, just let me know, and/or stop by irc :) |
Can you make it "go-ipfs-shell"? There will be other languages. These are basically bindings. |
Just some outsiders perspective on the pin/add issue: Without having seen your discussion here, I expected that |
If you don't pin the object you just freshly added, what's to keep these objects from vanishing a few seconds later? Nobody is interested in these object as nobody knows their hash. As the objects are not pinned, they are only stored temporarily, and risk being thown out of your temporary cache. |
Hi @mildred, thanks for your explanation. But there's some confusion on my side now: I did
Therefore, I assume that |
I'd expect it to |
@SCBuergel we made a call early on that matched an admittedly small-sample, but strong indicator that:
we just haven't added the |
Thanks, @jbenet. That default behavior was not initially clear to me. |
If we move to |
Closing this, initial concerns have been addressed |
ipfs pin ls, still doesnt show my recently added folders |
( |
while working with pinning to test out the gc command, i ran into a few issues that revolved around understanding the pinning system, for example:
I dont think its immediately apparent what the right thing to do to accomplish a given task is, the UX here needs some thought.
The text was updated successfully, but these errors were encountered: