-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: add store/get
& upload/get
capabilities
#82
Conversation
These capabilities are intended for checking inclusion and allowing users to verify all the shards for an upload have been removed when removing an upload. see also: storacha/w3up#942 License: MIT Signed-off-by: Oli Evans <[email protected]>
License: MIT Signed-off-by: Oli Evans <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine but we should update all of these to not use strings for CIDs....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could update these post-merge of course, but I do think these changes need doing
|
||
Executing a `store/get` invocation with a service provider should return a response object. | ||
|
||
If a failure occurs, the response will have an `error` field with a value of `true`, and a `message` string field with details about the error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe I'm misunderstanding this, but won't the error response look like:
{ error: { name: "ErrorName", message: "An error message" } }
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yes you right. I was blending this in with the rest of this spec, but things have changed!
I can't find a place in our other specs where we document the expected output shapes which seems odd too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeaaaa I agree, I think it's a hole in our specs that we haven't documented the input/return types across the board, maybe even a DX issue worth fixing asap?
|
||
If a failure occurs, the response will have an `error` field with a value of `true`, and a `message` string field with details about the error. | ||
|
||
On success, the response object will have the following shape: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and similarly, I think the response will look like:
{ ok: {link: "bagy...", size: 42, insertedAt: "2023-01-01T01:01:01" } }
w3-store.md
Outdated
|
||
#### Caveats | ||
|
||
The `root` caveat must contain the root CID of the data item to remove. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/remove/return/
|
||
| `field` | `value` | `required?` | `context` | | ||
| --------- | --------------------------------- | ----------- | ------------------------------------------------------------- | | ||
| `root` | data CID string, e.g. `bafy...` | | The root CID of the upload to get metadata for | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
required should be true I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, but i think we need to make it clearer that these tables are referring to "when used as an invocation", as the root
is not required in the schema, as folks should be allowed to make a delegation that grants you upload/get
on any root. same for store.get
. My gut feel is we should make the tables present the ucanto schema rather than the "when used as invocation" form.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(yep, as i deliberately differed from the other items in this spec where they have marked an optional field as required)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ahhhh got it, I agree re: presenting ucanto schema!!
|
||
Executing an `upload/get` invocation with a service provider should return a response object. | ||
|
||
If a failure occurs, the response will have an `error` field with a value of `true`, and a `message` string field with details about the error. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto re: response formats in this section
Capability definitions to allow clients to check for inclusion of cid in store shards or upload roots. Also allows client to fetch the additional properties we store. This will, for example, let a client verify all shards for an upload have been removed before removing an upload. _I'm aiming to simplify things with this PR so that we return insertedAt info on the Upload/Store ListItem types, and return the same object shape for a single object get. If this is pleasing, then I will continue in this direction and make Remove also return the same shape if it removes something_ TODO: - [x] spec update storacha/specs#82 - [x] tests License: MIT --------- Signed-off-by: Oli Evans <[email protected]> Co-authored-by: Alan Shaw <[email protected]>
implemented in storacha/w3up#1178 |
Adds the `blob/get` capability spec which is analogous to #82 --------- Co-authored-by: Vasco Santos <[email protected]> Co-authored-by: Alan Shaw <[email protected]>
These capabilities are intended for checking inclusion, or getting the additional properties we store. This will allow clients to verify all the shards for an upload have been removed before removing an upload.
store/get
delegationResponse
Error if
link
shard cid is not in space or...upload/get
delegationResponse
Error
if root is not in uploads for space or...see also: storacha/w3up#942
License: MIT