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

Clean up ClientConfig HTTP options #864

Open
anacrolix opened this issue Aug 29, 2023 · 3 comments
Open

Clean up ClientConfig HTTP options #864

anacrolix opened this issue Aug 29, 2023 · 3 comments

Comments

@anacrolix
Copy link
Owner

anacrolix commented Aug 29, 2023

There are a proliferation of HTTP related options in ClientConfig. I believe it might be possible to converge on just exposing *http.Client for each use case. *http.Client handles having custom proxies, TLS settings, redirect handling, timeouts etc.

The use cases for HTTP in anacrolix/torrent are:

  • Trackers
  • WebRTC
  • Sources (fetching torrent files)
  • Webseeds
  • ??

One strong holdout against this I think is the websocket library in use for webtorrent trackers. It only takes a proxy option. However the nyooyr.io/websocket module takes *http.Client, so this could be upgraded.

Another place is resolving IPs for trackers. This is currently done manually to check against blocklists before issuing HTTP requests. I think this is still doable, but some APIs might change.

@anacrolix
Copy link
Owner Author

I wonder if you might chime in with your thoughts @ccampbell, @yangrq1018.

@veshij
Copy link
Contributor

veshij commented Feb 10, 2024

I can try to work on this issue.
I suppose it'll require backward-incompatible changes of ClientConfig (deprecating existing fields).
Should we support both options for a while?

@anacrolix
Copy link
Owner Author

I can try to work on this issue. I suppose it'll require backward-incompatible changes of ClientConfig (deprecating existing fields). Should we support both options for a while?

I think examining all the places that could be affected, determining that it could work then rolling it out subsystem by subsystem should work fine if it is indeed the right abstraction for this.

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

No branches or pull requests

2 participants