From patchwork Fri Jul 20 11:57:22 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Russell Currey X-Patchwork-Id: 946843 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 41X8Xf1xgFz9s7c for ; Fri, 20 Jul 2018 21:58:10 +1000 (AEST) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=russell.cc Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.b="coxZPIlI"; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 41X8Xf0SwnzDqD3 for ; Fri, 20 Jul 2018 21:58:10 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=russell.cc Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.b="coxZPIlI"; dkim-atps=neutral X-Original-To: snowpatch@lists.ozlabs.org Delivered-To: snowpatch@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=none (mailfrom) smtp.mailfrom=russell.cc (client-ip=64.147.123.21; helo=wout5-smtp.messagingengine.com; envelope-from=ruscur@russell.cc; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=russell.cc Authentication-Results: lists.ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.b="coxZPIlI"; dkim-atps=neutral Received: from wout5-smtp.messagingengine.com (wout5-smtp.messagingengine.com [64.147.123.21]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 41X8XV3mVYzDqCG for ; Fri, 20 Jul 2018 21:58:01 +1000 (AEST) Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailout.west.internal (Postfix) with ESMTP id 57210385; Fri, 20 Jul 2018 07:57:56 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute6.internal (MEProxy); Fri, 20 Jul 2018 07:57:56 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:date:from:in-reply-to:message-id :references:subject:to:x-me-sender:x-me-sender:x-sasl-enc; s= fm3; bh=FdcY6tK7jik4YkYxoOUMrbjpnrn25/Lq7iO4MsKKBwU=; b=coxZPIlI D5y2DQ4C+A4kQTPfD8lYWXYhie8VRupk8QbB6fxuD/zZ/NjchA+O2zpwLTzfqvPQ SNMpLgPMD8WxbpPY6XX0FcE2FRu1JJ5tmzqaVQ8ShtRkT8H41DK7JVBsaK+nBxlO FXmrCRlmP65UzzpJG8z05K+QkTqfD1K0bzbqkXIiY0plWuztpoRoJZtjTlx121FT 6PbDW5e0awE10sI4lYZjSD8tfYcBVLXBVqXt/uBYEhthbTz61/WhkpcknwUDcrjX L6z2JEpeQf12ZLcjZMfMqDYkBAhLpdhcAFKHCQ5D+YJ8pTtYwVwtW+98JBOhjclt flvnnu/S4Xjwlg== X-ME-Proxy: X-ME-Sender: Received: from snap.ozlabs.ibm.com (unknown [122.99.82.10]) by mail.messagingengine.com (Postfix) with ESMTPA id 4FD0D10269; Fri, 20 Jul 2018 07:57:54 -0400 (EDT) From: Russell Currey To: snowpatch@lists.ozlabs.org, andrew.donnellan@au1.ibm.com Date: Fri, 20 Jul 2018 21:57:22 +1000 Message-Id: <20180720115728.19312-2-ruscur@russell.cc> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20180720115728.19312-1-ruscur@russell.cc> References: <20180720115728.19312-1-ruscur@russell.cc> Subject: [snowpatch] [PATCH 2/8] Require project argument to be specified for daemon mode X-BeenThere: snowpatch@lists.ozlabs.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: Continuous Integration for patch-based workflows List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Russell Currey MIME-Version: 1.0 Errors-To: snowpatch-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "snowpatch" So this might seem out of place, but bear with me here. Right now, the way that we check for patches is to get the latest patches "page" from the API. For each of them, we see if the project the patch is from matches one we have settings for, and if so we test it. We exhaust the patch list page we got, and then we get a new one. There are lots of problems with this. The first one is that if we're testing lots of patches and the Patchwork instance is high traffic, we risk missing patches that fall to page 2 or beyond in that time. We currently don't have a solution for this. The next issue is that when I originally implemented this, filtering by project didn't work properly in the patches API, making it difficult to track all patches, especially for niche projects with less activity (since running into one patch of a series will lead to them all being tested, even if snowpatch doesn't find them all individually in the patch list). This is no longer the case however, and we can filter by project on the patch list and actually get a full page of patches in that project. As a result, until both the following conditions are true: - patchwork has a suitable events API and/or better pagination - snowpatch has a proper threading model for distributed work Limit snowpatch to one project at a time to minimise the chance of missing patches, and users should just spin up more than one instance if they want to test multiple projects at once. Signed-off-by: Russell Currey Reviewed-by: Andrew Donnellan --- src/main.rs | 130 +++++++++++++++++++++-------------------------- src/patchwork.rs | 8 ++- 2 files changed, 64 insertions(+), 74 deletions(-) diff --git a/src/main.rs b/src/main.rs index 44bc013..5bf6187 100644 --- a/src/main.rs +++ b/src/main.rs @@ -67,8 +67,8 @@ mod utils; static USAGE: &'static str = " Usage: - snowpatch [--count=] [--project ] - snowpatch --mbox --project + snowpatch --project [--count=] + snowpatch --project --mbox snowpatch --patch snowpatch --series snowpatch -v | --version @@ -77,11 +77,11 @@ Usage: By default, snowpatch runs as a long-running daemon. Options: + --project Test patches for the given project. --count Run tests on recent series. --patch Run tests on the given Patchwork patch. --series Run tests on the given Patchwork series. --mbox Run tests on the given mbox file. Requires --project - --project Test patches for the given project. -v, --version Output version information. -h, --help Output this help text. "; @@ -349,19 +349,6 @@ fn main() { panic!("Can't specify both --series and --patch"); } - if args.flag_mbox != "" && args.flag_project != "" { - info!("snowpatch is testing a local patch."); - let patch = Path::new(&args.flag_mbox); - match settings.projects.get(&args.flag_project) { - None => panic!("Couldn't find project {}", args.flag_project), - Some(project) => { - test_patch(&settings, &client, project, patch, true); - } - } - - return; - } - if args.flag_patch > 0 { info!("snowpatch is testing a patch from Patchwork."); let patch = patchwork.get_patch(&(args.flag_patch as u64)).unwrap(); @@ -400,6 +387,17 @@ fn main() { return; } + // At this point, specifying a project is required + let project = settings.projects.get(&args.flag_project).unwrap(); + + if args.flag_mbox != "" { + info!("snowpatch is testing a local patch."); + let patch = Path::new(&args.flag_mbox); + test_patch(&settings, &client, project, patch, true); + + return; + } + // The number of patches tested so far. If --count isn't provided, this is unused. let mut patch_count = 0; @@ -411,7 +409,7 @@ fn main() { */ 'daemon: loop { let patch_list = patchwork - .get_patch_query() + .get_patch_query(&args.flag_project) .unwrap_or_else(|err| panic!("Failed to obtain patch list: {}", err)); info!("snowpatch is ready to test new revisions from Patchwork."); for patch in patch_list { @@ -426,72 +424,60 @@ fn main() { continue; } - //let project = patchwork.get_project(&patch.project).unwrap(); - // Skip if we're using -p and it's the wrong project - if args.flag_project != "" && patch.project.link_name != args.flag_project { - debug!( + // Skip if it's the wrong project + if patch.project.link_name != args.flag_project { + warn!( "Skipping patch {} ({}) (wrong project: {})", patch.name, patch.id, patch.project.link_name ); continue; } - match settings.projects.get(&patch.project.link_name) { - None => { - debug!( - "Project {} not configured for patch {}", - &patch.project.link_name, patch.name - ); - continue; - } - Some(project) => { - // TODO(ajd): Refactor this. - let hefty_tests; - let mbox = if patch.has_series() { - debug!( - "Patch {} has a series at {}!", - &patch.name, &patch.series[0].url - ); - let series = patchwork.get_series_by_url(&patch.series[0].url); - match series { - Ok(series) => { - if !series.received_all { - debug!("Series is incomplete, skipping patch for now"); - continue; - } - let dependencies = patchwork.get_patch_dependencies(&patch); - hefty_tests = dependencies.len() == series.patches.len(); - patchwork.get_patches_mbox(dependencies) - } - Err(e) => { - debug!("Series is not OK: {}", e); - hefty_tests = true; - patchwork.get_patch_mbox(&patch) - } + // TODO(ajd): Refactor this. + let hefty_tests; + let mbox = if patch.has_series() { + debug!( + "Patch {} has a series at {}!", + &patch.name, &patch.series[0].url + ); + let series = patchwork.get_series_by_url(&patch.series[0].url); + match series { + Ok(series) => { + if !series.received_all { + debug!("Series is incomplete, skipping patch for now"); + continue; } - } else { + let dependencies = patchwork.get_patch_dependencies(&patch); + hefty_tests = dependencies.len() == series.patches.len(); + patchwork.get_patches_mbox(dependencies) + } + Err(e) => { + debug!("Series is not OK: {}", e); hefty_tests = true; patchwork.get_patch_mbox(&patch) - }; - - let results = test_patch(&settings, &client, project, &mbox, hefty_tests); - - // Delete the temporary directory with the patch in it - fs::remove_dir_all(mbox.parent().unwrap()) - .unwrap_or_else(|err| error!("Couldn't delete temp directory: {}", err)); - if project.push_results { - for result in results { - patchwork.post_test_result(result, &patch.checks).unwrap(); - } - } - if args.flag_count > 0 { - patch_count += 1; - debug!("Tested {} patches out of {}", patch_count, args.flag_count); - if patch_count >= args.flag_count { - break 'daemon; - } } } + } else { + hefty_tests = true; + patchwork.get_patch_mbox(&patch) + }; + + let results = test_patch(&settings, &client, project, &mbox, hefty_tests); + + // Delete the temporary directory with the patch in it + fs::remove_dir_all(mbox.parent().unwrap()) + .unwrap_or_else(|err| error!("Couldn't delete temp directory: {}", err)); + if project.push_results { + for result in results { + patchwork.post_test_result(result, &patch.checks).unwrap(); + } + } + if args.flag_count > 0 { + patch_count += 1; + debug!("Tested {} patches out of {}", patch_count, args.flag_count); + if patch_count >= args.flag_count { + break 'daemon; + } } } info!("Finished testing new revisions, sleeping."); diff --git a/src/patchwork.rs b/src/patchwork.rs index d59fc40..378c6ca 100644 --- a/src/patchwork.rs +++ b/src/patchwork.rs @@ -300,8 +300,12 @@ impl PatchworkServer { serde_json::from_str(&self.get_url_string(url).unwrap()) } - pub fn get_patch_query(&self) -> Result, serde_json::Error> { - let url = format!("{}{}/patches/{}", &self.url, PATCHWORK_API, PATCHWORK_QUERY); + pub fn get_patch_query(&self, project: &str) -> Result, serde_json::Error> { + let url = format!( + "{}{}/patches/{}&project={}", + &self.url, PATCHWORK_API, PATCHWORK_QUERY, project + ); + serde_json::from_str(&self.get_url_string(&url) .unwrap_or_else(|err| panic!("Failed to connect to Patchwork: {}", err))) }