Skip to content

Add support for constructing custom proxy routing.#81

Open
jeeger wants to merge 6 commits into
babashka:mainfrom
jeeger:feature/parse-proxy-env-variables
Open

Add support for constructing custom proxy routing.#81
jeeger wants to merge 6 commits into
babashka:mainfrom
jeeger:feature/parse-proxy-env-variables

Conversation

@jeeger

@jeeger jeeger commented Apr 22, 2026

Copy link
Copy Markdown
  • Modified ->ProxySelector to take a vector of handlers and proxy parameters in addition to the "simple" host/port config argument.
  • Added functions to construct handler/proxy pairs.
  • Added some tests for proxy selection
  • Updated changelog.

Please answer the following questions and leave the below in as part of your PR.

- Modified `->ProxySelector` to take a vector of handlers and proxy parameters in addition to the "simple" host/port config argument.
- Added functions to construct handler/proxy pairs.
- Added some tests for proxy selection
- Updated changelog.
@borkdude

Copy link
Copy Markdown
Contributor

My initial reaction: why would we pass a vector of functions? I think we can cover all use cases with just one function right?

@jeeger

jeeger commented Apr 22, 2026

Copy link
Copy Markdown
Author

I'll give it a go and see how the one-function interface would look.

@borkdude

Copy link
Copy Markdown
Contributor

What I mean is, one could write all the conditions basically as one function with a cond instead of providing multiple separate functions, right?

@borkdude

borkdude commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Feebdack:

It seems ->ProxySelector now lost the ProxySelector passthrough (the case where you pass something that is already a ProxySelector). Perhaps lock this down in a test as well?

The default client has a behavior change now, it will suddenly respect env vars. I think this should be opt-in since this is breaking. Since java.net.http doesn't respect env vars by default, I think we should just expose a function for this.

Maybe we could do this as a second PR as to not make this PR contain too many features at once.

@jeeger

jeeger commented Apr 23, 2026

Copy link
Copy Markdown
Author

Good points, I'll split the "default proxy" behavior into its own pull request and add the "passthrough" case.

@jeeger jeeger marked this pull request as ready for review May 7, 2026 10:00
@jeeger

jeeger commented May 11, 2026

Copy link
Copy Markdown
Author

This is done now.

@jeeger

jeeger commented May 12, 2026

Copy link
Copy Markdown
Author

It helps to actually run the tests, fixed the test error.

@borkdude

Copy link
Copy Markdown
Contributor

Here's my feedback:

  • Let's not expose any new functions in this PR, e.g. ->Proxy
  • Maybe we could support :type in the static {:host :port} config too, so a static socks proxy is possible? (Optional)
  • I think we can move the dispatch to internal/->ProxySelector so (http/client {:proxy (fn [uri] ...)}) works?

- Removed `->Proxy` function from public interface, dynamic proxy functions now return a map of `:host`, `:port` and `:type`.
- Support `:type` for static proxy configuration.  Allows specifying a static socks proxy now.
- Moved proxy selector dispatch on argument type into internal package.
@jeeger

jeeger commented Jun 15, 2026

Copy link
Copy Markdown
Author

Applied the requested changes, I hope I interpreted everything correctly.

Comment thread src/babashka/http_client.clj
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.

2 participants