Skip to content
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 blob list and remove into spec #123

Merged
merged 3 commits into from
Apr 22, 2024

Conversation

vasco-santos
Copy link
Contributor

@vasco-santos vasco-santos commented Apr 18, 2024

This PR adds blob listing and removing to the spec as a first version. It is clear we will want to improve this later on, but we agreed in shipping this doing same behaviour as store/list and store/remove so that we do not lose functionality by shipping blob protocol by default.

With the above in mind, and as we agreed this PR basically ports the store/list and store/remove here, with some adaptions to be consistent with structure in this document, as well as usage of ts code snippets instead of ipldsch to also be consistent

@vasco-santos vasco-santos force-pushed the feat/add-blob-list-and-remove-into-spec branch from 3e79bb8 to 1abb587 Compare April 18, 2024 10:06
@vasco-santos vasco-santos marked this pull request as ready for review April 18, 2024 10:29
@vasco-santos vasco-santos force-pushed the feat/add-blob-list-and-remove-into-spec branch from 1cf174b to 55fc9b8 Compare April 18, 2024 15:19
w3-blob.md Outdated

##### List Pre

The optional `args.pre` field MAY be set to `true` to request a page of results preceding cursor. If `args.pre` is omitted or set to `false` provider MUST respond with a page following the specified `args.cursor`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we leave this out of the new spec? I'm not sure it's needed and I don't believe a widely supported feature in databases other than DynamoDB.

w3-blob.md Outdated
Comment on lines 583 to 588
"blob": {
// cursor where to start listing from
"cursor": 'cursor-value-from-previous-invocation',
// size of page
"size": 40,
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"blob": {
// cursor where to start listing from
"cursor": 'cursor-value-from-previous-invocation',
// size of page
"size": 40,
}
// cursor where to start listing from
"cursor": 'cursor-value-from-previous-invocation',
// size of page
"size": 40,

w3-blob.md Outdated
@@ -563,6 +565,231 @@ type LocationCommitment = {
}
```

## List Blob

Authorized agent MAY invoke `blob/list` capability on the [space] subject (`sub` field) to list Blobs added to it at the time of invocation.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we please be consistent with 1.0 naming i.e. this should be /space/content/list/blob and there should be a note for what we're using in the current version.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh damn! Yes, sorry this is a typo, I will make it consistent with the 1.0 naming and let a follow up PR to document the current version for all the spec

results: ListBlobItem
}

type ListBlobItem = {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't want to complicate things here so it's not a request, more of suggestion with an expectation that we'll probably do it sometime in the future instead.

I think we should stop having lists like and instead just return list of receipts instead, we can let client deal with getting / caching receipts locally and deriving timestamps from when receipt was issued.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, actually @alanshaw and I also talked about shifting into that direction later on, and use more the pattern you described recently on a RFC ("our databases should be considered more an index over our receipts"). But for MVP let's make it like store and iterate on this when possible

@vasco-santos vasco-santos merged commit 929c7bb into main Apr 22, 2024
2 checks passed
@vasco-santos vasco-santos deleted the feat/add-blob-list-and-remove-into-spec branch April 22, 2024 10:02
vasco-santos added a commit to storacha/w3up that referenced this pull request Apr 23, 2024
Implements storacha/specs#123

BREAKING CHANGE: allocations storage interface now requires remove to be
implemented
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants