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
fix!: conform to Delegated Routing V1 HTTP spec #41
fix!: conform to Delegated Routing V1 HTTP spec #41
Changes from 8 commits
294eeb7
c14818d
ec104c4
df60a7f
622971e
2a958bc
10d77e2
1dd9e70
a0859dc
36222eb
fbeca5e
7fc45c1
b14ff94
ab5e8fd
1115ac8
4176ca9
77b096b
14fb52c
60171a6
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.
Can the return type be guaranteed to be
BitswapRecord
?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.
@achingbrain we don't want BitswapRecords to be used, see https://specs.ipfs.tech/routing/http-routing-v1/#get-routing-v1-providers-cid - it should be a generic record. PeerRecords are recommended.
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.
Then can the return type be guaranteed to be
PeerRecord
? What I'm getting at here is it makes the users life easier if they don't have to deal with a union type.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.
There could be other schemas than
peer
. But in practice we can limit this toPeerRecord
responses: ignore unknown schemas unless they haveID
andAddrs
fields like the legacybitswap
schema, and interpret them aspeer
schema.For the wider context, the only
/routing/v1
implementation (afaik) that returned bitswap schema was cid.contact (IPNI), and it hadID
andAddrs
just like peer schema does.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.
Ok, sounds like we can just convert any
BitswapRecord
s toPeerRecord
s and drop the union type then.