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(http-routers): filter-protocols from IPIP-484 #173

Merged
merged 1 commit into from
Oct 16, 2024
Merged

Conversation

lidel
Copy link
Member

@lidel lidel commented Oct 15, 2024

This is Rainbow version of ipfs/kubo#10534 which adds support for filter-protocols from IPIP-484 and makes Rainbow use unknown + transport-bitswap as implicit default.

The intention here is to skip peers which are not actionable.

@lidel lidel requested a review from gammazero October 15, 2024 23:27
delegatedRouter, err := delegatedHTTPContentRouter(endpoint,
routingv1client.WithHTTPClient(httpClient),
routingv1client.WithProtocolFilter(cfg.RoutingV1FilterProtocols), // IPIP-484
routingv1client.WithStreamResultsRequired(), // https://specs.ipfs.tech/routing/http-routing-v1/#streaming
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of scope for this PR, but...

According to the docs,

server MAY respond with non-streaming application/json response even if the client requested streaming

So, would his option be better named WithStreamResultsRequested or WithStreamResultsAccepted?

Copy link
Member Author

@lidel lidel Oct 16, 2024

Choose a reason for hiding this comment

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

Yes, imo the name is confusing and redundant.

The default accepts both here, and specs says you should always return JSON as fallback anyway.
(afaik this config was onl yadded because cid.contact errored on streaming requests, but it was fixed since then)

@lidel lidel merged commit c20ca3a into main Oct 16, 2024
13 checks passed
@lidel lidel deleted the feat/ipip-484 branch October 16, 2024 15:31
@lidel lidel mentioned this pull request Oct 16, 2024
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.

2 participants