v0.0.2: Pinning Summit + initial feedback

[lidel]

This version is following spec drafted during Pinning Summit where we switched API to operate on "pin objects" with some improvements based on initial feedback and discussions received from the community this week.

This is by no means the finished spec: goal is to close some topics and kick-off discussions on remaining items.

Changes in this PR

❓ – needs clarification or further discussion
⁉️– potentially controversial

• Changes from Pinning Summit, namely change API to operate on "pin objects" (closes Apply feedback from Pinning Summit #2)
  • Replaced vendors-specific fields such as replicationParam with a free form meta
• Basic pagination with skip and limit parameters (closes Pagination for returning large pinsets #12)
• Added ability to filter pin objects using arrays of CIDs and/or statuses (closes Ability to check status of multiple CIDs at once #1)
• ❓ simplified authorization based on opaque string that can be passed in HTTP header or query param, as suggested by @mikeal in Defining API access controls #6 (comment)
• Examples and missing descriptions
• ❓ Status enum in which pin object can exist at a pinning service
  (for now those are just strings)
• ⁉️ replaced "pin type" with more flexible depth
  It can represent direct (0) and recursive (-1) pins just fine, but opens more advanced strategies in the future.
• ⁉️ GET /pins/?cid=Qm&depth=1 could return pin status of arbitrary subtree depth is just a simple filter (direct/recursive)

Documentation Preview

Docs for feat/pinning-summit-variant-v0.0.2 branch can be viewed at:

• https://bafybeibarvujzcxuqikzdb22pzdbza4hz4rdvaagrcunjs7zhaqamw46ku.ipfs.dweb.link/

cc @pooja @jacobheun @aschmahmann @achingbrain @autonome @Gozala @momack2 @olizilla
@obo20 @sanderpick

[obo20]

⁉️ replaced "pin type" with more flexible depth
It can represent direct (0) and recursive (-1) pins just fine, but opens more advanced strategies in the future.
⁉️ GET /pins/?cid=Qm&depth=1 could return pin status of arbitrary subtree
(open question: how to represent indirect pins in PinStatus object? add indirect Status enum?)

I'm not sure if filtering based on pin depth is possible right now. As far as I'm aware all current pinning services (including Pinata) only track pinned items by the root CID.

[lidel]

@obo20 fair enough, could be revisited in the future, if needed.

For now, I believe we should be ok with simplified depth filter (recursive/direct).
As long cid filter accepts an array of CIDs to provide a way of addressing need described in #1

[aschmahmann]

This overall seems fine to me, although I've got a few thoughts/questions about pinning service flow and the pinning objects we're introducing here.

• My concern is that if users are expecting something like ipfs pin add --with-service=mypinner QmXYZ to immediately ship the data to the pinning service that we could run into problems if QmXYZ is not already published to the DHT
• This becomes worse with the pinning objects since we'll generate a new pin object that we'll have to wait for the DHT to finish providing before the pinning service can find the data. We'll likely want to not even send the HTTP request until the DHT put is completed
  • We can work around this by pre-connecting to a pinning service upload IPFS node, if that's available, but since my understanding was that having one of those was not a requirement for implementing this API it's a shame that users may have to wait for a DHT provide to finish before the pinning can start
  • If we expect users to have many different pin objects that they'll be providing to the network they could get bogged down in the provide queue. How many pin objects do we expect an "average" user to create if they are pinning N objects?
  • If we are able to send the pin object over HTTP then this problem basically goes away

[lidel]

@aschmahmann good catch! I failed to document that aspect, fixed now.

Pinned data may not be present at a pinning service and at the same time pin object creation should not be a blocking operation. User may be pinning big data set that takes time to transfer and we don't want CLI and other tools to hang. Instead of waiting for remote pinning to finish, cid-of-pin-object is returned immediately in PinStatus response, and can be used for periodical status checks later.

I've added Pin object lifecycle to the spec and missing PinStatus responses required for async status checks of ongoing pinning operations.

Pin status/progress check can be done via GET /pins/{cid-of-pin-object} or GET /pins?cid={cid-being-pinned}.

Q: how specific should the status reporting be?

Right now Status is an enum string with values like "pinning" vs "pinned", but I wonder if ability to return % value would be more flexible / useful for UIs here. Thoughts?

[achingbrain]

Could this be recursive by default so depth is only used if it's specified?

We wouldn't need to deal with depth: -1 then, which just looks a bit weird.

[obo20]

Is there a reason we need to filter based on depth? Having a recursive / direct depth filter wouldn't be possible on our end right now. I don't think any pinning service that currently exists keeps track of anything other than the root CID.

[lidel]

Removed filter, and based on feedback (nobody supports depth other than 0 and -1) I am thinking about removing depth field too and replacing it with recursive boolean which is true by default.

Will fill a separate PR after this one is merged.

[jacobheun]

Right now Status is an enum string with values like "pinning" vs "pinned", but I wonder if ability to return % value would be more flexible / useful for UIs here. Thoughts?

I'd skip this for now. This could come back in the meta object for pins that are in progress.

[jacobheun]

Should we add the minimum(1), maximum(1000?) and default(max?) here? While services could vary I think it would be good to just lock this in for simplicity.

[obo20]

I think these are sane values, I would advocate a default of 10 just for simplicity sake.

[lidel]

@obo20 I worry 10 may be too low, making default the same as max decreases the number of things people need to think about. If you feel strongly about this, open issue or PR against master after this one is merged, and we can discuss changing the default.

[obo20]

Q: how specific should the status reporting be?
Right now Status is an enum string with values like "pinning" vs "pinned", but I wonder if ability to return % value would be more flexible / useful for UIs here. Thoughts?

I don't think % pinned is currently possible, but we could certainly provide statuses that signify the object is currently being fetched. For reference, this are the current statuses we have in Pinata's system when pinning by CID.

• "prechecking" - Pinata is running preliminary validations on your pin request.
• "searching" - Pinata is actively searching for your content on the IPFS network. This may take some time if your content is isolated.
• "retrieving" - Pinata has located your content and is now in the process of retrieving it.
• "expired" - Pinata wasn't able to find your content after a day of searching the IPFS network. Please make sure your content is hosted on the IPFS network before trying to pin again.
• "over_free_limit" - Pinning this object would put you over the free tier limit. Please add a credit card to continue pinning content.
• "over_max_size" - This object is too large of an item to pin. If you're seeing this, please contact us for a more custom solution.
• "invalid_object" - The object you're attempting to pin isn't readable by IPFS nodes. Please contact us if you receive this, as we'd like to better understand what you're attempting to pin.
• "bad_host_node" - You provided a host node that was either invalid or unreachable. Please make sure all provided host nodes are online and reachable.

Obviously not all of these would apply to every service, but I do think it would be good to have a standard set of statuses that we can all agree on.

[obo20]

I think these are sane values, I would advocate a default of 10 just for simplicity sake.

[lidel]

Addressed today's feedback from this PR and workshop with Core Impl WG.
I don't believe anything really controversial is left,
this PR is big enough, we can tackle the rest in standalone issues/PRs.

👉 I am going to merge this so it is easier to open issues/PRs for remaining things and discuss them in parallel.

[lidel]

Removed filter, and based on feedback (nobody supports depth other than 0 and -1) I am thinking about removing depth field too and replacing it with recursive boolean which is true by default.

Will fill a separate PR after this one is merged.

[lidel]

@obo20 I worry 10 may be too low, making default the same as max decreases the number of things people need to think about. If you feel strongly about this, open issue or PR against master after this one is merged, and we can discuss changing the default.