| Message ID | 1462766571-7834-1-git-send-email-ruscur@russell.cc |
|---|---|
| State | Changes Requested |
| Delegated to: | Andrew Donnellan |
| Headers | show |
On Mon, 2016-05-09 at 14:02 +1000, Russell Currey wrote: > With the new HTTP proxy support in hyper 0.9.2, we can finally include it > in snowpatch. > > Enable snowpatch to use HTTP proxies through the http_proxy environment > variable, as seems to be standard. > > Signed-off-by: Russell Currey <ruscur@russell.cc> > --- Another idea would be to have individual proxy settings per server, which might come up since Jenkins and Patchwork could reside in different networks. This would be too painful at the moment where proxy support in hyper is defined when the Client is created (which for us is only once). I think they'll expand it further at some point though, and when they do we can look at implementing that.
On 09/05/16 14:02, Russell Currey wrote: > With the new HTTP proxy support in hyper 0.9.2, we can finally include it > in snowpatch. Excellent! > > Enable snowpatch to use HTTP proxies through the http_proxy environment > variable, as seems to be standard. What about https? > + let client = Arc::new(match env::var("http_proxy") { > + Ok(mut proxy) => { > + if proxy.starts_with("http://") { > + proxy.drain(..7); > + } > + let mut port = 80; > + if let Some(colon) = proxy.rfind(":") { > + port = proxy[colon + 1..].parse().unwrap_or_else(|e| { > + panic!("http_proxy is malformed: {:?}, error: {}", proxy, e); > + }); > + proxy.truncate(colon); > + } > + Client::with_http_proxy(proxy, port) > + }, > + _ => Client::new() > + }); Can we do the URL parsing with the url module instead to avoid doing string awfulness? Something along the lines of: (completely untested) let client = Arc::new(match env::var("http_proxy") { Ok(proxy_url) => { let proxy = Url::parse(proxy_url).unwrap_or_else(|e| { panic!("Malformed URL: {:?}, error: {}", proxy_url, e); assert!(proxy.scheme() == "http"); assert!(proxy.path() == ""); Client::with_http_proxy(proxy.host_str(), if proxy.port() { proxy.port() } else { 80 }) }, _ => Client::new() });
On Mon, 2016-05-16 at 10:52 +1000, Andrew Donnellan wrote: > On 09/05/16 14:02, Russell Currey wrote: > > With the new HTTP proxy support in hyper 0.9.2, we can finally include it > > in snowpatch. > > Excellent! > > > > > Enable snowpatch to use HTTP proxies through the http_proxy environment > > variable, as seems to be standard. > > What about https? I don't know. The ::with_http_proxy thing is all that's in hyper from what I can tell, whether that'll actually happily use a https proxy I don't know. It does make successful https connections through a http proxy from what I can tell..I'll try to confirm. > > > + let client = Arc::new(match env::var("http_proxy") { > > + Ok(mut proxy) => { > > + if proxy.starts_with("http://") { > > + proxy.drain(..7); > > + } > > + let mut port = 80; > > + if let Some(colon) = proxy.rfind(":") { > > + port = proxy[colon + 1..].parse().unwrap_or_else(|e| { > > + panic!("http_proxy is malformed: {:?}, error: {}", > > proxy, e); > > + }); > > + proxy.truncate(colon); > > + } > > + Client::with_http_proxy(proxy, port) > > + }, > > + _ => Client::new() > > + }); > > Can we do the URL parsing with the url module instead to avoid doing > string awfulness? > > Something along the lines of: (completely untested) > > let client = Arc::new(match env::var("http_proxy") { > Ok(proxy_url) => { > let proxy = Url::parse(proxy_url).unwrap_or_else(|e| { > panic!("Malformed URL: {:?}, error: {}", proxy_url, e); > assert!(proxy.scheme() == "http"); > assert!(proxy.path() == ""); > Client::with_http_proxy(proxy.host_str(), if proxy.port() { > proxy.port() } else { 80 }) > }, > _ => Client::new() > }); This is a very good idea that I'm upset I didn't think of.
diff --git a/Cargo.toml b/Cargo.toml index 614abbb..2055e8a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -26,7 +26,7 @@ license = "GPL-2.0+" name = "snowpatch" [dependencies] -hyper = "0.9" +hyper = ">0.9.2" # 0.9.2 or later required for proxy support git2 = "0.3" rustc-serialize = "0.3" url = "0.5" diff --git a/src/main.rs b/src/main.rs index e9c2035..25899a4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -38,6 +38,7 @@ use std::sync::Arc; use std::thread; use std::time::Duration; use std::path::Path; +use std::env; mod patchwork; use patchwork::{PatchworkServer, TestState, TestResult}; @@ -226,7 +227,22 @@ fn main() { let settings = settings::parse(args.arg_config_file); // The HTTP client we'll use to access the APIs - let client = Arc::new(Client::new()); + let client = Arc::new(match env::var("http_proxy") { + Ok(mut proxy) => { + if proxy.starts_with("http://") { + proxy.drain(..7); + } + let mut port = 80; + if let Some(colon) = proxy.rfind(":") { + port = proxy[colon + 1..].parse().unwrap_or_else(|e| { + panic!("http_proxy is malformed: {:?}, error: {}", proxy, e); + }); + proxy.truncate(colon); + } + Client::with_http_proxy(proxy, port) + }, + _ => Client::new() + }); let mut patchwork = PatchworkServer::new(&settings.patchwork.url, &client); if settings.patchwork.user.is_some() {
With the new HTTP proxy support in hyper 0.9.2, we can finally include it in snowpatch. Enable snowpatch to use HTTP proxies through the http_proxy environment variable, as seems to be standard. Signed-off-by: Russell Currey <ruscur@russell.cc> --- Cargo.toml | 2 +- src/main.rs | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-)