From f9b13604484fa7924efb8cde0dd26de0276065ff Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Tue, 14 Nov 2017 23:46:27 +0100 Subject: find_and_track_build_and_update_status(): Move thread outside function Instead of wrapping the inside of the function in a thread, put the thread on the outside, in `main()`. This is an effort to try to get around the `move`/lifetime issues I was having with the previous version. Add some extra logging so we have a better idea what's going on when. Remove the `expects` I had added and put the `Result` returns back. In `main()`, wrap the call to `find_and_track_build_and_update_status()` in a thread and sleep before calling it. This allows us to give Jenkins some time to warm up and create a job for our commit before we go ahead and try to request it from its API. Otherwise, if we kick off the Jenkins fetch too soon, our statuses aren't going to get updated because the job won't have been created and thus won't have been found on the `get_jobs()` call. Had to add some ugly `clone()`s to get around `move` compiler errors here. Next step is figuring out how to clean that up. --- src/main.rs | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) (limited to 'src/main.rs') diff --git a/src/main.rs b/src/main.rs index 05ca775..c93d616 100644 --- a/src/main.rs +++ b/src/main.rs @@ -25,7 +25,9 @@ extern crate rouille; extern crate kipper; use std::env; +use std::thread; use std::io::Read; +use std::time::Duration; use getopts::Options; @@ -137,20 +139,30 @@ fn main() { }, }; - match jenkins::find_and_track_build_and_update_status( - commit_ref, - jenkins_url.clone(), - &jenkins_user_id, - &jenkins_token, - github_token.clone(), - ) { - Ok(_) => {}, - Err(e) => { - error!("{}", e.to_string()); - - return internal_server_error() - }, - }; + // TODO: Get rid of clones + let jenkins_url = jenkins_url.clone(); + let jenkins_user_id = jenkins_user_id.clone(); + let jenkins_token = jenkins_token.clone(); + let github_token = github_token.clone(); + + thread::spawn(move || { + thread::sleep(Duration::from_secs(30)); + + match jenkins::find_and_track_build_and_update_status( + commit_ref, + &jenkins_url, + &jenkins_user_id, + &jenkins_token, + &github_token, + ) { + Ok(_) => {}, + Err(e) => { + error!("{}", e.to_string()); + + // return internal_server_error() + }, + }; + }); rouille::Response::text("202 Accepted") .with_status_code(202) -- cgit v1.2.3 From 84d4e100e93a5e3b630eaa355120871af46667d2 Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Thu, 16 Nov 2017 23:39:15 +0100 Subject: main(): Add comment above cloned command line variables Turns out the easiest way to share these variables between both closures really is to clone them. Thanks to 'durka42' on Mozilla#rust-beginners for explaining the problem to me. In durka's words: > both closures are required to be 'static > so you can't send references across > and the router handler has to be callable multiple times so you can't consume anything (that's the error you got) It's also possible to share references using Arc, but that seems more complicated than it's worth here. Just do the copy and be done with it. --- src/main.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/main.rs') diff --git a/src/main.rs b/src/main.rs index c93d616..8c4fbfb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -139,7 +139,9 @@ fn main() { }, }; - // TODO: Get rid of clones + // Clone so we can use these values in the thread + // closure. Since both closures are required to be + // 'static, we can't use references to these values. let jenkins_url = jenkins_url.clone(); let jenkins_user_id = jenkins_user_id.clone(); let jenkins_token = jenkins_token.clone(); -- cgit v1.2.3 From 90ef027f2f36a8cb321b2dcaa8a411425338c200 Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Thu, 16 Nov 2017 23:45:36 +0100 Subject: find_and_track_build_and_update_status(): Restore old arg types I had changed these when I was having trouble passing the values through my double-closure, but now that that's settles, we can revert these types to the way they were before and get rid of the unnecessary `to_owned()` transformations. --- src/main.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/main.rs') diff --git a/src/main.rs b/src/main.rs index 8c4fbfb..211a680 100644 --- a/src/main.rs +++ b/src/main.rs @@ -152,10 +152,10 @@ fn main() { match jenkins::find_and_track_build_and_update_status( commit_ref, - &jenkins_url, + jenkins_url, &jenkins_user_id, &jenkins_token, - &github_token, + github_token, ) { Ok(_) => {}, Err(e) => { -- cgit v1.2.3 From 01fa452209dcea101809fd1b224e029ae4f4a9d9 Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Thu, 16 Nov 2017 23:53:23 +0100 Subject: main(): Remove commented `return` This error handler now lives inside a `thread::spawn()` rather than inside the HTTP request handler, so we shouldn't return an HTTP response here. --- src/main.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'src/main.rs') diff --git a/src/main.rs b/src/main.rs index 211a680..6bb4594 100644 --- a/src/main.rs +++ b/src/main.rs @@ -158,11 +158,7 @@ fn main() { github_token, ) { Ok(_) => {}, - Err(e) => { - error!("{}", e.to_string()); - - // return internal_server_error() - }, + Err(e) => error!("{}", e.to_string()), }; }); -- cgit v1.2.3