Skip to content

Fix RestClient proxy path to preserve ProxyManager behavior#221

Closed
NguyenHoangSon96 with Copilot wants to merge 3 commits into
refactor/rest-clientfrom
copilot/fix-code-review-comment
Closed

Fix RestClient proxy path to preserve ProxyManager behavior#221
NguyenHoangSon96 with Copilot wants to merge 3 commits into
refactor/rest-clientfrom
copilot/fix-code-review-comment

Conversation

Copilot AI commented Jul 13, 2026

Copy link
Copy Markdown

This PR addresses the single review thread requesting a fix for proxy handling in RestClient. The previous implementation replaced ProxyManager with a host-scoped connection pool, which bypassed manager-level proxy behavior (CONNECT/proxy headers).

  • What changed

    • Keep manager instances intact
      • RestClient.__init__ now stores urllib3.ProxyManager / urllib3.PoolManager directly instead of calling .connection_from_url(...).
    • Send full URLs through the manager
      • RestClient.request(...) now builds an absolute URL from base_url + path (for relative paths) and sends that URL via pool_manager.request(...).
      • Absolute URLs remain supported as-is.
  • Targeted test coverage

    • Proxy behavior regression test
      • Added a focused unit test to assert proxied RestClient keeps the manager instance and issues requests with an absolute URL.
rest = RestClient(base_url="http://localhost:8181", proxy="http://proxy:3128")
rest.request(method="GET", url="/ping")
# manager.request called with: ("GET", "http://localhost:8181/ping", ...)

Copilot AI changed the title [WIP] Fix code for review comment Fix RestClient proxy path to preserve ProxyManager behavior Jul 13, 2026
Copilot AI requested a review from NguyenHoangSon96 July 13, 2026 11:48
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