routing/http: feat: add streaming support #18
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ajnavarro
previously requested changes
Jan 3, 2023
guseggert
force-pushed
the
feat/routing-http-iterator
branch
2 times, most recently
from
013e55a to
b24943a
Compare
ajnavarro
reviewed
Jan 17, 2023
ajnavarro
reviewed
Jan 17, 2023
Codecov Report
@@ Coverage Diff @@
## main #18 +/- ##
==========================================
+ Coverage 47.79% 48.03% +0.24%
==========================================
Files 274 279 +5
Lines 33237 33439 +202
==========================================
+ Hits 15885 16063 +178
- Misses 15673 15694 +21
- Partials 1679 1682 +3
|
masih
reviewed
Jan 26, 2023
guseggert
force-pushed
the
feat/routing-http-iterator
branch
4 times, most recently
from
498cfce to
f21e4b7
Compare
|
@guseggert : what's remaining so this can move out of draft? |
guseggert
force-pushed
the
feat/routing-http-iterator
branch
from
7e82960 to
4dea63f
Compare
This was referenced Feb 21, 2023
Closed
aschmahmann
reviewed
Mar 20, 2023
Jorropo
referenced
this pull request
in ipfs/go-libipfs-rapide
Mar 23, 2023
This commit was moved from ipfs/go-verifcid@c45e93b
Closed
|
2023-05-09 maintainer conversation:
|
This adds streaming support to the routing/v1 client and server by changing the interfaces to use iterators instead of slices, and adding content type negotation to the client and server.
Errors can just be packed inside of a struct, it over-complicates the interface to require errors as separate return values everywhere.
This is always needed, so we can simplify things by just including it in the main iterator interface.
These are practically the same, but cid.contact has already started using ndjson so we're just going to use that instead.
guseggert
force-pushed
the
feat/routing-http-iterator
branch
from
93b5f90 to
296fcef
Compare
|
Re: testing with cid.contact, I talked with @masih and I was wrong--this is not implemented in cid.contact yet. It is for IPNI APIs but not for routing/v1. Confirmed by wiring into Kubo, req Accept=application/x-ndjson,application/json but resp Content-Type=application/json. Tracking issue for this on their side: ipni/indexstar#92 I think the best option is to just go ahead and ship this in Kubo and let cid.contact own the integration testing when they are ready to deploy. |
guseggert
force-pushed
the
feat/routing-http-iterator
branch
from
12c75c2 to
d21f0c8
Compare
hacdias
approved these changes
May 10, 2023
5 tasks
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This adds streaming support to the routing/v1 client and server by changing the interfaces to use iterators instead of slices, and adding content type negotation to the client and server with
application/x-ndjsonsupport.