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
IPIP-293: Add /ipld Gateway Specs #293
base: main
Are you sure you want to change the base?
IPIP-293: Add /ipld Gateway Specs #293
Changes from all commits
b9d46c4
2488113
8a07dcc
5cc59f2
3674381
8e59a2b
4c9e3ad
a471eb4
b7e48a3
c96abcd
0372363
5723b06
447118c
c447f90
b140bdd
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.
Spec should remove any ambiguity:
Location
header?text/plain
?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.
This is something I'd like to clarify with @fabricedesre since we had a bit of a disagreement.
Right now the precendent within Kubo and Agregore's protocol handlers is that there will be a 201 response with a
Location
header containing the URL as well as an empty body.Fabrice was into having a 200 response and the URL inside the response body, which is something I was originally doing in Agregore, but switch when we started extending the writable gateway functionality in Kubo.
Ideally we should settle on the best course of action here during Lisbon. 😅
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.
Ideally, I'd like to use this to inform all the other protocol handlers 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.
Do we have use cases where things other than
localhost
could be used in the future?e.g. do we want to support POST to IPNS identifier?
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've been using
POST
toipfs://localhost
, or aPUT ipfs://cid/
as well asPOST ipns://key
to update CIDs, orPUT ipns://key
in the Agregore IPFS Daemon SpecThere 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.
What if data is a valid JSON (and not DAG-JSON) added to ipfs with
json
codec (and notdag-json
)?Parsing it as dag-json will error, even tho it is a valid JSON.
@hacdias and I discussed this edge case and ended up with requirement to check codec from CID, and if it is
json
, use generic JSON codec instead ofdag-json
.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.
Interesting. Do you have text written up somewhere that I can copy paste here?
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.
@RangerMauve we have some wording here, but it may not be definitive:
specs/http-gateways/PATH_GATEWAY.md
Lines 184 to 187 in 67fab21
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.
TODO: link to IANA when ipfs/in-web-browsers#202 is resolved
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.
TODO: link to IANA when ipfs/in-web-browsers#201 is resolved
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.
nit: I've read this section and tbh have no idea what is the value to end user – CBOR traversal and field resolution with extra steps so the output looks a certain way?
In ADL section we have good use case "ADL that's used to represent large maps" – we need similar real world example for schemas.
What would a schema be useful for irl? I feel the spec here needs better
Example
, so the value is obvious.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.
@rvagg @warpfork would you be able to comment on real world uses of IPLD Schema that would be relevant here?
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.
One use case for schemas is to use the
representation
functionality to render data in more human/application readable formats from formats that are more size efficient.e.g. some things might be using a
listpairs
representation which would look like an array of arrays by default. But with a schema you can transform the representation to be more human readable.I'm gonna be doing stuff along this line for the Prolly Tree work where we'll be encoding tree nodes more efficiently, but having a way to put them through a schema before the application code starts working with them.
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.
nit: was unable to inspec this via
ipfs dag get --output-codec=dag-json bafyreibvheoym4avfsjfw63yhsymovm7o54ftcnxwxovqf5xxcbjddanze | jq
As a rule of thumb, CIDs used in IPIP should be publicly available and pinned (e.g. to https://estuary.tech and https://web3.storage, do not use Pinata as afaik it does not announce CIDs on DHT).
We will have automation for this, btu for now it is up to IPIP author to handle.
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.
Should I maybe include some CBOR files with the fixtures that are relevant to the spec?