Skip to content

Expose URLLIB connection pool KW - #1008

Closed
romanzdk wants to merge 2 commits into
massive-com:masterfrom
romanzdk:master
Closed

romanzdk wants to merge 2 commits into
massive-com:masterfrom
romanzdk:master

Conversation

@romanzdk

@romanzdk romanzdk commented Mar 18, 2026 •

Copy link
Copy Markdown
Contributor

Add option to specify urllib kwargs. We would use it e.g. for max size.

@romanzdk

Copy link
Copy Markdown
Contributor Author

Solves #1009 (comment)

@romanzdk

romanzdk commented Apr 6, 2026

Copy link
Copy Markdown
Contributor Author

@penelopus would you have a few minutes to check, please?

@justinpolygon

Copy link
Copy Markdown
Collaborator

@romanzdk I'll look at this today. I just want to make sure we're not going to break backwards compatibility or the way people are connecting today. From my tests is looks fairly safe but anything that touches the way we initialize the client I want to be careful of.

@romanzdk

Copy link
Copy Markdown
Contributor Author

Hey @justinpolygon , have you had a chance to take a look at this, please? :)

@romanzdk

Copy link
Copy Markdown
Contributor Author

kind reminder @justinpolygon

@justinpolygon
justinpolygon removed the request for review from penelopus August 7, 2026 20:25
justinpolygon added a commit that referenced this pull request Oct 16, 2026
Combines the community PRs that change how the REST client builds its
urllib3 pool (#968, #1008, #1026, #1033, #1034).

- CA bundle: both clients verify TLS with ca_certs if given, then the
  SSL_CERT_FILE or REQUESTS_CA_BUNDLE environment variable, then
  certifi (#998).
- RESTClient(maxsize=10): connections kept per host. urllib3 defaults
  to 1, so threaded clients kept opening new TLS connections.
- RESTClient(connection_pool_kw=...): other urllib3 pool options,
  merged over the defaults (#1009).
- RESTClient(proxy=...): explicit proxy URL, with credentials from the
  URL. Proxy environment variables aren't read, so existing traffic
  doesn't change route. The WebSocket client already uses the system
  proxy via websockets.

Co-authored-by: SACHU JAMES <142034811+SachuJames@users.noreply.github.com>
Co-authored-by: Mohd Quamar Tyagi <104281681+Tyagiquamar@users.noreply.github.com>
Co-authored-by: jakobovski-arb <201493996+jakobovski-arb@users.noreply.github.com>
Co-authored-by: Roman Zydyk <31843161+romanzdk@users.noreply.github.com>
Co-authored-by: Kevin Chiu <2712+kevinchiu@users.noreply.github.com>
justinpolygon added a commit that referenced this pull request Oct 16, 2026
Adds connection options to the clients, combining #968, #1008, #1026,
#1033 and #1034 (contributors are credited as co-authors).

- **Custom CA:** `ca_certs=` on `RESTClient` and `WebSocketClient`,
defaulting to `SSL_CERT_FILE` / `REQUESTS_CA_BUNDLE`, then certifi.
Closes #998.
- **Pool size:** `RESTClient(maxsize=10)`. urllib3 defaults to 1
connection per host.
- **Pool options:** `RESTClient(connection_pool_kw={...})`, merged over
the defaults. Closes #1009.
- **Proxy:** `RESTClient(proxy="http://user:pass@host:port")`. Proxy env
vars aren't read, so nobody's traffic changes route.

The README has a new **Network Configuration** section with examples for
each case (custom CA, proxy, threads, other pool options).

All new parameters are optional and added at the end of the signatures.
The one behavior change: if `SSL_CERT_FILE` or `REQUESTS_CA_BUNDLE`
points to a file, it's now used instead of certifi (as `requests` and
Python do). If it points to a file that doesn't exist, it's skipped with
a warning, so a stale value doesn't break anyone.

Verified against the live API through a local TLS-inspecting proxy
(mitmproxy with auth): REST and WebSocket connect with the proxy's CA
(argument or env var) and fail certificate verification without it;
wrong proxy credentials get 407. Under 16 threads, `maxsize=10` opened
16 TLS connections vs 57–70 with urllib3's default.

---------

Co-authored-by: SACHU JAMES <142034811+SachuJames@users.noreply.github.com>
Co-authored-by: Mohd Quamar Tyagi <104281681+Tyagiquamar@users.noreply.github.com>
Co-authored-by: jakobovski-arb <201493996+jakobovski-arb@users.noreply.github.com>
Co-authored-by: Roman Zydyk <31843161+romanzdk@users.noreply.github.com>
Co-authored-by: Kevin Chiu <2712+kevinchiu@users.noreply.github.com>
@justinpolygon

Copy link
Copy Markdown
Collaborator

Thanks, and sorry this sat so long! connection_pool_kw is in #1040 (you're credited as a co-author), released in v2.9.0. Your options take precedence over the defaults, and maxsize is also a direct argument now. Closing in favor of #1040.

@massive-com massive-com deleted a comment from penelopus Oct 16, 2026
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