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

Extract go-trustless-utils; and more improvements discovered along the way #404

Merged
merged 9 commits into from
Sep 8, 2023

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Aug 31, 2023

Primary things now in https://github.com/ipld/go-trustless-utils are:

  • "verifiedcar" package from here is now called "traversal" over there
  • misc http utilities that are also useful in Frisbii and should be generally useful
  • basic types, DagScope, ByteRange and a Request type has been extracted that's a subset of what's now called RetrievalRequest here; the difference being that Request represents a Trustless Gateway request, while RetrievalRequest has lots of lassie extras on it.

Other things done in here, much of which was discovered while doing the extraction:

  • removes "car-scope" handling
  • minor changes to selector building for entity-bytes non-entity dag-scope
  • improved error messages
  • refactored traversal out of bitswapretriever
  • major refactor/cleanup-reorg in /ipfs/ daemon handler
  • return "bytes" in "Accept-Ranges" header (not "none")
  • check dups parameter in content-type of HTTP remotes and handle accordingly

Includes some of the work I did in #386 around the traversal stuff — including collecting the LastPath so it can be dealt with elsewhere. I'm not currently using it here but I am using it in Frisbii to log cases where the requested path couldn't be completed in the DAG; I'll do the same here soon too for the CLI and close #386 out.

@codecov-commenter
Copy link

codecov-commenter commented Aug 31, 2023

Codecov Report

Merging #404 (cf96aa6) into main (768f669) will decrease coverage by 1.23%.
The diff coverage is 91.08%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #404      +/-   ##
==========================================
- Coverage   77.00%   75.77%   -1.23%     
==========================================
  Files          87       84       -3     
  Lines        6700     6228     -472     
==========================================
- Hits         5159     4719     -440     
+ Misses       1267     1258       -9     
+ Partials      274      251      -23     
Files Changed Coverage Δ
pkg/types/types.go 74.60% <ø> (-5.06%) ⬇️
cmd/lassie/fetch.go 31.84% <20.00%> (ø)
pkg/retriever/parallelpeerretriever.go 92.00% <50.00%> (ø)
pkg/retriever/retriever.go 87.75% <62.50%> (ø)
pkg/server/http/ipfs.go 89.31% <93.57%> (+1.58%) ⬆️
pkg/internal/testutil/mockroundtripper.go 87.92% <94.73%> (+2.28%) ⬆️
pkg/internal/itest/testpeer/peerhttpserver.go 68.93% <100.00%> (ø)
pkg/internal/testutil/gen.go 64.12% <100.00%> (-26.72%) ⬇️
pkg/retriever/assignablecandidatefinder.go 100.00% <100.00%> (ø)
pkg/retriever/bitswapretriever.go 96.01% <100.00%> (+1.61%) ⬆️
... and 3 more

... and 5 files with indirect coverage changes

📢 Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=filecoin-project).

@rvagg
Copy link
Member Author

rvagg commented Sep 7, 2023

Updated to ipld/go-trustless-utils#6 (will need to update again on tag)

* removes "car-scope" handling
* minor changes to selector building for entity-bytes non-entity dag-scope
* improved error messages
* refactored traversal out of bitswapretriever
* major refactor/cleanup-reorg in /ipfs/ daemon handler
* return "bytes" in "Accept-Ranges" header
@rvagg rvagg merged commit 36ebbef into main Sep 8, 2023
7 checks passed
@rvagg rvagg deleted the rvagg/trustless-utils branch September 8, 2023 05:36
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.

3 participants