-
Notifications
You must be signed in to change notification settings - Fork 97
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(gateway): more explicit IPFSBackend and no multi-range #369
Conversation
Codecov Report
@@ Coverage Diff @@
## main #369 +/- ##
==========================================
- Coverage 66.44% 65.75% -0.69%
==========================================
Files 204 208 +4
Lines 24621 25099 +478
==========================================
+ Hits 16360 16505 +145
- Misses 6838 7140 +302
- Partials 1423 1454 +31
|
@aschmahmann can we not support multiple ranges? Isn't that part of the specs? Also note that I'll be working on IPIP-412 soon, sow e might get some conflicts. |
db5809e
to
1cffe61
Compare
1cffe61
to
5bca4fc
Compare
2023-07-11 maintainer conversation: we shouldn't stress about multiple ranges. We have a metric for this against ipfs.io and haven't found any actual usage. This conformance test for multiple ranges can be moved to Kubo. |
5bca4fc
to
0a58918
Compare
cd21e88
to
a8d5f27
Compare
@BigLep @aschmahmann I don't understand how that helps. If we move the test to Kubo, it will still fail since we are removing the functionality on this PR and that functionality will simply no longer be available. |
Correct. IIUC the right thing to do here is:
We can add back in the multiple range functionality, but IIUC from @lidel basically no one is using this so it's not worth the hassle at the moment. |
Working on updating the tests here: ipfs/gateway-conformance#113 |
b2a3b77
to
90fccc2
Compare
@aschmahmann @lidel this PR is blocked until ipfs/gateway-conformance#113 is merged and released (ready for review), so that we can have the tests passing here and/or in Kubo's PR. |
1b58ebf
to
6f95ad1
Compare
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.
(submitting first-pass at the end of day)
50fcc4e
to
34b4055
Compare
34b4055
to
35865d5
Compare
35865d5
to
592fafc
Compare
2023-08-22 conversation: @lidel will review and figure out appropriate testing story (Boxo unit tests vs. conformance tests) |
2023-08-24 conversation: |
@aschmahmann : is this on track to land this week? |
2023-09-12 conversation:
|
b7b9648
to
1d2b5c4
Compare
…ing single range requests
Co-authored-by: Marcin Rataj <lidel@lidel.org>
1d2b5c4
to
dcd7a19
Compare
CI is green in Kubo. As per what was agreed, I'm merging this once CI finishes here and bubbling it up. |
Also, switches to only handling single range requests rather than multiples.