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

routing/http: add more type and WithDynamicProviderInfo #443

Closed
wants to merge 2 commits into from

Conversation

Jorropo
Copy link
Contributor

@Jorropo Jorropo commented Aug 21, 2023

This is required to fix a bug in Kubo where we advertise the listening addresses.
We can't pass our real addresses once since thing like autonat takes time.

@Jorropo Jorropo requested a review from a team as a code owner August 21, 2023 13:10
@Jorropo Jorropo changed the title Http client types routing/http: add more type and WithDynamicProviderInfo Aug 21, 2023
@codecov
Copy link

codecov bot commented Aug 21, 2023

Codecov Report

Merging #443 (a97f44f) into main (db4e43a) will decrease coverage by 0.61%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #443      +/-   ##
==========================================
- Coverage   50.11%   49.50%   -0.61%     
==========================================
  Files         249      246       -3     
  Lines       29998    29950      -48     
==========================================
- Hits        15033    14827     -206     
- Misses      13532    13687     +155     
- Partials     1433     1436       +3     
Files Changed Coverage Δ
routing/http/contentrouter/contentrouter.go 51.19% <33.33%> (ø)
routing/http/client/client.go 76.49% <71.42%> (-1.34%) ⬇️
routing/http/internal/drjson/json.go 75.00% <100.00%> (ø)

... and 17 files with indirect coverage changes

@hacdias
Copy link
Member

hacdias commented Aug 22, 2023

@Jorropo please note that routing/http is suffering immense changes in #422.

@Jorropo
Copy link
Contributor Author

Jorropo commented Aug 22, 2023

@hacdias this is a pretty simple change that adds a callback instead of statically hardcoding the listen addresses, I would be surprised if this can't be forward ported.

@hacdias
Copy link
Member

hacdias commented Aug 25, 2023

@Jorropo feel free to rebase and ping me after.

@Jorropo
Copy link
Contributor Author

Jorropo commented Aug 25, 2023

I'll probably do that for Kubo v0.24

@BigLep
Copy link
Contributor

BigLep commented Oct 5, 2023

2023-10-05 conversation: we aren't going to invest time in this until /routing/v1 PUTs get addressed: ipfs/specs#378. We aren't spending time fixing bugs here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants