Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
docs: datawherehouse location claim + store/publish #13
docs: datawherehouse location claim + store/publish #13
Changes from 2 commits
e59f282
78ace25
0972f75
7a20c7f
e779e93
07a6fa9
f1cf689
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I think this is not correct anymore, right?
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.
I think fact that size of the content / blob is not captured (or is optional) is an unfortunate oversight. I would suggest changing
range
field into required that way size of the content is clear from the claim.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.
I'm starting to think that private location claims are obsolete. Content claim should include whatever metadata it needs in the claim to make anyone with it able to perform a read. We can simply leverage UCAN auth system for the rest.
In other words query params ≈ ucan auth header. Later in addition gives us ability to choose who can exercise it while former is public by default.
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.
since we are all in agreement to not use this, I will just drop it from the RFC. FWIW, this is currently there as "alternative", but later is specified what this RFC proposes
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.
We should make this required field so size of the content can be inferred from the claim.
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.
well, for this case we are meaning blobs (or CAR files), which in theory is the entire thing, but yeah, since we need to do a HEAD request to see if it is in the write target we can make this required as it is really no extra cost