sneako / finch

Elixir HTTP client, focused on performance
MIT License
1.23k stars 114 forks source link

Support for pools based on a name rather than URL schema #246

Closed tomasz-tomczyk closed 8 months ago

tomasz-tomczyk commented 8 months ago

Hi, this is more of a question/feature request (and I'm happy to try to take it on if it makes sense), but I was wondering about your stance on supporting custom pools when the URL of the request is unknown, but I'd prefer not to bundle it all into the :default pool.

Use case: a service that implements HTTP requests for user-defined URLs (e.g. IFTTT with webhooks). For known HTTP clients, we can specify a pool per URL, then use :default as a catch-all for anything in our codebase we've missed, but we would like to steer those user-defined requests to its own pool.

Let me know your thoughts or if I've missed how that might be solved with the current options - thank you!

oliveigah commented 8 months ago

I'm not sure the understanding is clear when you say: "we can specify a pool per URL, then use :default as a catch-all for anything in our codebase we've missed, but we would like to steer those user-defined requests to its own pool."

Requests to previously unkowns URLs will not use the same pool. In fact it just use the default as a base configuration for new pools that will be started dynamically. So the "default" is not a pool per se, but just a configuration template to start new pools on demand. As the documentation says:

Pools will be started for each configured {scheme, host, port} when Finch is started. For any unconfigured {scheme, host, port}, the pool will be started the first time it is requested. https://hexdocs.pm/finch/Finch.html#module-usage

and

or the atom :default to provide a catch-all configuration to be used for any unspecified URLs. See "Pool Configuration Options" below for details on the possible map values. Default value is %{default: [size: 50, count: 1]}. https://hexdocs.pm/finch/Finch.html#start_link/1-options

I did not use Finch in a while but had the same use case as you when I implemented the idle timeout for http1 pools. So I may be wrong about the current state of the library (correct me here please @sneako :pray:) but at my current understanding you do not really have a problem here and can just tweak your default configuration properly.

tomasz-tomczyk commented 8 months ago

Ah, it appears I missed and/or misunderstood that! Thanks for your feedback ❤️