Repository navigation
Migrate ureq from 2 to 3 - #637
Open
yangyang-duolingo wants to merge 2 commits into
Open
yangyang-duolingo wants to merge 2 commits into
yangyang-duolingo wants to merge 2 commits into
Conversation
Gets the Python and uv runtime downloads native HTTP_PROXY/HTTPS_PROXY/ ALL_PROXY and NO_PROXY support for free, with zero buildpack-side proxy handling code: ureq 3's default Agent config sets proxy: Proxy::try_from_env() unconditionally (src/config.rs Config::default()), and that now includes NoProxy::try_from_env() natively (src/proxy.rs) -- unlike ureq 2, where proxy detection was gated behind an opt-in Cargo feature that didn't exist by default, and even with it on, had no NO_PROXY support at all (see heroku#630). Three mechanical API changes, no logic changes: - ureq::Error::Status(code, _) -> ureq::Error::StatusCode(code) (no longer carries the Response). - response.into_reader() -> response.into_body().into_reader() (.call() now returns http::Response<Body> instead of ureq 2's own Response type). - Cargo.toml's ureq feature "tls" -> "rustls" (renamed upstream). Added a test that proves this at the socket level rather than just asserting on config: a fake local HTTP proxy observes a CONNECT tunnel request for a target not covered by NO_PROXY, and observes nothing at all for a request whose target *is* covered by NO_PROXY (confirming it went directly to the real target instead). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #636 (and makes #630/#631's
http_agent()workaround unnecessary —ureq3's defaultAgentconfig now honoursHTTP_PROXY/HTTPS_PROXY/ALL_PROXY/NO_PROXYnatively, with no opt-in feature flag needed at all).Three mechanical API changes, no logic changes:
ureq::Error::Status(code, _)→ureq::Error::StatusCode(code)(no longer carries theResponse) — one match arm insrc/layers/python.rs.response.into_reader()→response.into_body().into_reader()(.call()now returnshttp::Response<Body>instead ofureq2's ownResponsetype) — the two download functions insrc/utils.rs.Cargo.toml'sureqfeature"tls"→"rustls"(renamed upstream).Added a test (
ureq_default_agent_honours_http_proxy_and_no_proxy) that proves the proxy/NO_PROXYbehavior at the socket level rather than just asserting on config: a fake local HTTP proxy and a fake direct target, withHTTP_PROXY/NO_PROXYenv vars set, confirm that a request to a target not covered byNO_PROXYarrives at the proxy as aCONNECT <host>:<port>tunnel (ureq 3 tunnels every proxied request viaCONNECT, even for a plainhttp://target), while a request to a target that is covered byNO_PROXYlands directly on the real target and never reaches the proxy at all.Open questions for maintainers
ureq3 requires Rust 1.85; this buildpack already requires 1.99, so no conflict here, but flagging in case there's a constraint I'm not aware of.ureq3 is a from-scratch rewrite beyond proxying (newhttp-crate-based API throughout) — worth a wider look at whether anything else in this crate's usage needs attention beyond the three call sites here.Verification steps
cargo test(all 48 non-Docker-dependent tests pass, including the new one)cargo clippy --all-targets -- -D warnings(clean)cargo fmt --check(clean)cargo test -- --ignored) — not run here, no Docker/Pack available in my environment