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.

Sign up for GitHub

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

Jump to bottom

feat(gateway): implement IPIP-402 extensions for gateway CAR requests #303

Merged
merged 3 commits into from
Jun 8, 2023
Merged

feat(gateway): implement IPIP-402 extensions for gateway CAR requests #303

merged 3 commits into from
Jun 8, 2023

Conversation

aschmahmann
Copy link
Contributor

@aschmahmann aschmahmann commented May 16, 2023 •
edited
Loading

@aschmahmann aschmahmann force-pushed the feat/gateway-ipip-402 branch 2 times, most recently from 0b2dac0 to 8f39b1a Compare May 16, 2023 13:34
aschmahmann
aschmahmann commented May 16, 2023
View reviewed changes
gateway/gateway.go Outdated Show resolved Hide resolved
Jorropo
Jorropo reviewed May 16, 2023
View reviewed changes
gateway/blocks_gateway.go Outdated Show resolved Hide resolved
Jorropo
Jorropo reviewed May 16, 2023
View reviewed changes
gateway/blocks_gateway.go Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented May 16, 2023 •
edited
Loading

Codecov Report

Merging #303 (7be4474) into main (a85e005) will decrease coverage by 0.08%.
The diff coverage is 56.82%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #303      +/-   ##
==========================================
- Coverage   50.15%   50.08%   -0.08%     
==========================================
  Files         281      278       -3     
  Lines       33763    33983     +220     
==========================================
+ Hits        16935    17019      +84     
- Misses      15046    15173     +127     
- Partials     1782     1791       +9
Impacted Files Coverage Δ
examples/gateway/car/main.go 11.53% <0.00%> (ø)
examples/gateway/proxy/main.go 0.00% <0.00%> (ø)
gateway/dns.go 0.00% <ø> (ø)
gateway/handler_ipns_record.go 18.46% <0.00%> (-2.23%) ⬇️
gateway/handler_unixfs__redirects.go 37.63% <36.36%> (ø)
gateway/blocks_backend.go 44.38% <44.38%> (ø)
gateway/handler_defaults.go 40.26% <50.00%> (+1.85%) ⬆️
gateway/handler_unixfs_dir.go 64.28% <50.00%> (ø)
gateway/metrics.go 75.00% <69.69%> (-0.22%) ⬇️
gateway/handler_car.go 64.88% <74.68%> (+6.55%) ⬆️
... and 9 more

... and 12 files with indirect coverage changes

@aschmahmann aschmahmann force-pushed the feat/gateway-ipip-402 branch 2 times, most recently from a400896 to 42f28cb Compare May 16, 2023 14:23
aschmahmann
aschmahmann commented May 16, 2023
View reviewed changes
gateway/blocks_gateway.go Outdated Show resolved Hide resolved
gateway/blocks_gateway.go Outdated Show resolved Hide resolved
gateway/blocks_gateway.go Outdated Show resolved Hide resolved
gateway/blocks_gateway.go Outdated Show resolved Hide resolved
hacdias
hacdias reviewed May 16, 2023
View reviewed changes
gateway/handler_car.go Outdated Show resolved Hide resolved
aschmahmann
aschmahmann commented May 16, 2023
View reviewed changes
gateway/handler.go Outdated Show resolved Hide resolved
@lidel lidel mentioned this pull request May 16, 2023
Merged
aschmahmann
aschmahmann commented May 16, 2023
View reviewed changes
gateway/handler_car.go Outdated Show resolved Hide resolved
@hacdias hacdias mentioned this pull request May 17, 2023
Merged
@hacdias hacdias mentioned this pull request May 17, 2023
Closed
lidel
lidel reviewed May 17, 2023
View reviewed changes
gateway/handler_car.go Outdated Show resolved Hide resolved
gateway/handler.go Outdated Show resolved Hide resolved
gateway/handler_car.go Outdated Show resolved Hide resolved
@aschmahmann aschmahmann mentioned this pull request May 19, 2023
Merged
5 tasks
lidel
lidel reviewed May 29, 2023
View reviewed changes
gateway/handler_car.go Outdated Show resolved Hide resolved
gateway/handler_car.go Outdated Show resolved Hide resolved
gateway/handler_car.go Outdated Show resolved Hide resolved
gateway/handler_car.go Outdated Show resolved Hide resolved
@hacdias hacdias force-pushed the feat/gateway-ipip-402 branch 6 times, most recently from 7647b01 to cdc0436 Compare June 1, 2023 09:02
@rvagg rvagg mentioned this pull request Jun 2, 2023
Merged
@hacdias hacdias force-pushed the feat/gateway-ipip-402 branch 2 times, most recently from fe573f5 to 817991c Compare June 2, 2023 08:50
@hacdias hacdias mentioned this pull request Jun 2, 2023
Merged
@hacdias hacdias requested a review from a team as a code owner June 2, 2023 14:17
@hacdias hacdias requested a review from lidel June 2, 2023 14:53
@hacdias hacdias force-pushed the feat/gateway-ipip-402 branch 7 times, most recently from 4544260 to 07a0bef Compare June 5, 2023 09:24
@hacdias hacdias mentioned this pull request Jun 7, 2023
Merged
lidel
lidel previously requested changes Jun 7, 2023
View reviewed changes
gateway/handler_car.go Outdated Show resolved Hide resolved
gateway/handler_car_test.go Show resolved Hide resolved
gateway/blocks_backend.go Outdated Show resolved Hide resolved
@hacdias hacdias force-pushed the feat/gateway-ipip-402 branch 3 times, most recently from df19eb1 to a93ce13 Compare June 8, 2023 08:59
aschmahmann and others added 3 commits June 8, 2023 16:48
@aschmahmann @hacdias
feat(gateway): implement IPIP-402 extensions for gateway CAR requests
ac5c706 
Co-authored-by: Henrique Dias <[email protected]>
@hacdias
refactor: namings, fields, clarity, comments, documentation
ffcdaa1 
- Improve documentation of public API that to conform with pkg.go.dev
  stylings. Can be previewed with:
    `go install golang.org/x/pkgsite/cmd/pkgsite@latest && pkgsite`
    Note that there's a bug that makes the comments in struct fields not
    formatted: golang/go#56004
- Renamed a few things for consistency and clarity:
    - `BlocksGateway` to `BlocksBackend` (it is an `IPFSBackend`)
    - `api` to `backend` (it is an `IPFSBackend`)
    - `WithHostname` to `NewHostnameHandler`
    - `Specification` to `PublicGateway`
    - `ErrorResponse` to `ErrorStatusCode`
    - `DagEntityByteRange` to `DagByteRange` (there isn't other)
- Unexported `ServeContent`, which was never meant to be public
- Removed `webRequestError`, which is already handled by `webError`.
  Added a test to ensure that is the case.
- Removed return types from functions that always returned nil.
- Exported `DagScopeEntity`, `DagScopeAll`, `DagScopeBlock` as they are `DagScope`
- Change some usages of global `log` variable to request-scoped `logger` for more context
- Handle CAR before maybe resolving path (CAR handles it differently)
- Response formats as constants (avoids typos, etc)
@hacdias
feat: use last segment cid as root
7be4474 
- this also allows to return the PathMetadata, which also
allows to set the correct X-Ipfs-Root header.
@hacdias
Copy link
Member

hacdias commented Jun 8, 2023

Plan is:

  1. @hacdias Merge this, update in Kubo.
  2. @laurentsenta merge conformance, release conformance.
  3. @hacdias release Boxo update.

@hacdias hacdias merged commit 475d576 into main Jun 8, 2023
@hacdias hacdias deleted the feat/gateway-ipip-402 branch June 8, 2023 15:18
@BigLep
Copy link
Contributor

BigLep commented Jun 8, 2023

2023-06-08 maintainer conversation:
Expanding #303 (comment)
Adin will handle the proactive communication to bedrock about what is happening.
@hacdias intends to do a 0.9.1 Boxo release on 2023-06-09 with this work included.

We'll let the spec conversation around CAR roots drive if there are any CAR spec changes and ipld/go-car updates required.

@aschmahmann aschmahmann mentioned this pull request Aug 14, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jorropo Jorropo Jorropo left review comments

@rvagg rvagg rvagg left review comments

@hacdias hacdias hacdias approved these changes

@lidel lidel lidel left review comments

Assignees

@aschmahmann aschmahmann

@hacdias hacdias

Labels
None yet
Projects
No open projects
IPFS Shipyard Team
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Boxo gateway should support IPIP-402 parameters
6 participants
@aschmahmann @hacdias @BigLep @lidel @rvagg @Jorropo