From d5f33a9a986f21c8b660eb1d3faa9953f2b8bba7 Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Tue, 14 Nov 2017 23:24:04 +0100 Subject: find_and_track_build_and_update_status(): Try to wrap the fn in a thread Try to put the whole function inside a thread. This doesn't work because of move problems. The reason for wrapping things in a thread is so I can add a `sleep` at the top of the function. We want to sleep before even making our first request to Jenkins because it seems like we're making the request too early, before the job has been created. This prevents the GitHub status from being set. --- src/jenkins.rs | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) (limited to 'src/jenkins.rs') diff --git a/src/jenkins.rs b/src/jenkins.rs index 61d3bf1..d507f00 100644 --- a/src/jenkins.rs +++ b/src/jenkins.rs @@ -97,17 +97,28 @@ pub fn find_and_track_build_and_update_status( jenkins_user_id: &String, jenkins_token: &String, github_token: String, -) -> Result<(), Box> { +// ) -> Result<(), Box> { +// ) -> thread::JoinHandle { +) -> thread::JoinHandle<()> { + thread::spawn(move || { + // Wait for Jenkins to warm up + sleep(Duration::from_secs(10)); + let jenkins_client = jenkins_request_client( &jenkins_user_id, &jenkins_token - )?; + ).expect("Failed to initialise Jenkins HTTP client"); let jobs = get_jobs( &jenkins_url, &jenkins_client, commit_ref.repo.as_ref() - )?; + ).expect( + format!( + "Failed to request Jenkins jobs for {}", + commit_ref.repo + ).as_ref() + ); let t20_minutes = 60 * 20; for job_url in jobs { @@ -115,7 +126,9 @@ pub fn find_and_track_build_and_update_status( &jenkins_url, &jenkins_client, job_url.as_ref() - )?; + ).expect( + format!("Failed to request Jenkins job {}", job_url).as_ref() + ); // Does `displayName` match if job_for_commit(&job, &commit_ref) { @@ -206,11 +219,13 @@ pub fn find_and_track_build_and_update_status( } }); - return Ok(()) + // return Ok(()) + return } } + }) - Ok(()) + // Ok(()) } pub fn auth_credentials(user_id: String, token: String) -> header::Basic { -- cgit v1.2.3 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/jenkins.rs | 57 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 28 insertions(+), 29 deletions(-) (limited to 'src/jenkins.rs') diff --git a/src/jenkins.rs b/src/jenkins.rs index d507f00..d61e7d0 100644 --- a/src/jenkins.rs +++ b/src/jenkins.rs @@ -43,7 +43,6 @@ extern crate reqwest; extern crate url; use std::error::Error; -use std::thread; use std::thread::sleep; use std::time::{Duration, Instant}; @@ -93,46 +92,46 @@ impl Job { pub fn find_and_track_build_and_update_status( commit_ref: CommitRef, - jenkins_url: String, - jenkins_user_id: &String, - jenkins_token: &String, - github_token: String, -// ) -> Result<(), Box> { -// ) -> thread::JoinHandle { -) -> thread::JoinHandle<()> { - thread::spawn(move || { - // Wait for Jenkins to warm up - sleep(Duration::from_secs(10)); + // jenkins_url: String, + // jenkins_user_id: &String, + // jenkins_token: &String, + // github_token: String, + jenkins_url: &str, + jenkins_user_id: &str, + jenkins_token: &str, + github_token: &str, +) -> Result<(), Box> { + // TODO: Remove this, rest should take same type as arguments maybe + let jenkins_url = jenkins_url.to_owned(); + let jenkins_user_id = &jenkins_user_id.to_owned(); + let jenkins_token = &jenkins_token.to_owned(); + let github_token = github_token.to_owned(); let jenkins_client = jenkins_request_client( &jenkins_user_id, &jenkins_token - ).expect("Failed to initialise Jenkins HTTP client"); + )?; let jobs = get_jobs( &jenkins_url, &jenkins_client, commit_ref.repo.as_ref() - ).expect( - format!( - "Failed to request Jenkins jobs for {}", - commit_ref.repo - ).as_ref() - ); + )?; let t20_minutes = 60 * 20; for job_url in jobs { + info!("Looking for job: {}", job_url); + let mut job = request_job( &jenkins_url, &jenkins_client, job_url.as_ref() - ).expect( - format!("Failed to request Jenkins job {}", job_url).as_ref() - ); + )?; // Does `displayName` match if job_for_commit(&job, &commit_ref) { - thread::spawn(move || { + info!("Job found: {}", job_url); + // thread::spawn(move || { // Start timer let now = Instant::now(); @@ -165,6 +164,8 @@ pub fn find_and_track_build_and_update_status( // call github::update_commit_status // stop + info!("Waiting for job to finish"); + if now.elapsed().as_secs() == t20_minutes { github::update_commit_status( &github_token, @@ -182,7 +183,7 @@ pub fn find_and_track_build_and_update_status( ).as_ref() ); - return + return Ok(()) } sleep(Duration::from_secs(30)); @@ -212,20 +213,18 @@ pub fn find_and_track_build_and_update_status( ).as_ref() ); - return + return Ok(()) } job = updated_job; } - }); + // }); - // return Ok(()) - return + return Ok(()) } } - }) - // Ok(()) + Ok(()) } pub fn auth_credentials(user_id: String, token: String) -> header::Basic { -- 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/jenkins.rs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) (limited to 'src/jenkins.rs') diff --git a/src/jenkins.rs b/src/jenkins.rs index d61e7d0..038056a 100644 --- a/src/jenkins.rs +++ b/src/jenkins.rs @@ -92,21 +92,11 @@ impl Job { pub fn find_and_track_build_and_update_status( commit_ref: CommitRef, - // jenkins_url: String, - // jenkins_user_id: &String, - // jenkins_token: &String, - // github_token: String, - jenkins_url: &str, - jenkins_user_id: &str, - jenkins_token: &str, - github_token: &str, + jenkins_url: String, + jenkins_user_id: &String, + jenkins_token: &String, + github_token: String, ) -> Result<(), Box> { - // TODO: Remove this, rest should take same type as arguments maybe - let jenkins_url = jenkins_url.to_owned(); - let jenkins_user_id = &jenkins_user_id.to_owned(); - let jenkins_token = &jenkins_token.to_owned(); - let github_token = github_token.to_owned(); - let jenkins_client = jenkins_request_client( &jenkins_user_id, &jenkins_token -- cgit v1.2.3 From 4f8f9fc7497d62385cc4d82c36892587703352f0 Mon Sep 17 00:00:00 2001 From: Teddy Wing Date: Thu, 16 Nov 2017 23:48:38 +0100 Subject: find_and_track_build_and_update_status(): Remove commented thread::spawn This `thread::spawn()` call was moved outside the function and into `main()`. We can remove it now. --- src/jenkins.rs | 161 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 80 insertions(+), 81 deletions(-) (limited to 'src/jenkins.rs') diff --git a/src/jenkins.rs b/src/jenkins.rs index 038056a..273e26c 100644 --- a/src/jenkins.rs +++ b/src/jenkins.rs @@ -121,94 +121,93 @@ pub fn find_and_track_build_and_update_status( // Does `displayName` match if job_for_commit(&job, &commit_ref) { info!("Job found: {}", job_url); - // thread::spawn(move || { - // Start timer - let now = Instant::now(); - - let commit_status = job.result.commit_status(); - - github::update_commit_status( - &github_token, - &commit_ref, - &commit_status, - job_url.clone(), - None, - "continuous-integration/jenkins".to_owned() - ).expect( - format!( - "GitHub pending status update failed for {}/{} {}.", - commit_ref.owner, - commit_ref.repo, - commit_ref.sha - ).as_ref() - ); - while job.result == JobStatus::Pending { - // loop - // if timer > 20 minutes - // call github::update_commit_status with timeout error - // return - // wait 30 seconds - // call request_job again - // if the status is different - // call github::update_commit_status - // stop - - info!("Waiting for job to finish"); - - if now.elapsed().as_secs() == t20_minutes { - github::update_commit_status( - &github_token, - &commit_ref, - &github::CommitStatus::Error, - job_url.clone(), - Some("The status checker timed out.".to_owned()), - "continuous-integration/jenkins".to_owned() - ).expect( - format!( - "GitHub timeout error status update failed for {}/{} {}.", - commit_ref.owner, - commit_ref.repo, - commit_ref.sha - ).as_ref() - ); - - return Ok(()) - } + // Start timer + let now = Instant::now(); + + let commit_status = job.result.commit_status(); + + github::update_commit_status( + &github_token, + &commit_ref, + &commit_status, + job_url.clone(), + None, + "continuous-integration/jenkins".to_owned() + ).expect( + format!( + "GitHub pending status update failed for {}/{} {}.", + commit_ref.owner, + commit_ref.repo, + commit_ref.sha + ).as_ref() + ); + + while job.result == JobStatus::Pending { + // loop + // if timer > 20 minutes + // call github::update_commit_status with timeout error + // return + // wait 30 seconds + // call request_job again + // if the status is different + // call github::update_commit_status + // stop + + info!("Waiting for job to finish"); + + if now.elapsed().as_secs() == t20_minutes { + github::update_commit_status( + &github_token, + &commit_ref, + &github::CommitStatus::Error, + job_url.clone(), + Some("The status checker timed out.".to_owned()), + "continuous-integration/jenkins".to_owned() + ).expect( + format!( + "GitHub timeout error status update failed for {}/{} {}.", + commit_ref.owner, + commit_ref.repo, + commit_ref.sha + ).as_ref() + ); + + return Ok(()) + } - sleep(Duration::from_secs(30)); + sleep(Duration::from_secs(30)); - let updated_job = request_job( - &jenkins_url, - &jenkins_client, - job_url.as_ref() + let updated_job = request_job( + &jenkins_url, + &jenkins_client, + job_url.as_ref() + ).expect( + format!("Failed to request job '{}'.", job_url).as_ref() + ); + + if job.result != updated_job.result { + github::update_commit_status( + &github_token, + &commit_ref, + &job.result.commit_status(), + job_url.clone(), + None, + "continuous-integration/jenkins".to_owned() ).expect( - format!("Failed to request job '{}'.", job_url).as_ref() + format!( + "GitHub status update failed for {}/{} {}.", + commit_ref.owner, + commit_ref.repo, + commit_ref.sha + ).as_ref() ); - if job.result != updated_job.result { - github::update_commit_status( - &github_token, - &commit_ref, - &job.result.commit_status(), - job_url.clone(), - None, - "continuous-integration/jenkins".to_owned() - ).expect( - format!( - "GitHub status update failed for {}/{} {}.", - commit_ref.owner, - commit_ref.repo, - commit_ref.sha - ).as_ref() - ); - - return Ok(()) - } - - job = updated_job; + return Ok(()) } - // }); + + job = updated_job; + } return Ok(()) } -- cgit v1.2.3