From c3bf2dec7b277c2ad18a3eff5972743299fecdd3 Mon Sep 17 00:00:00 2001 From: Kaspar Tint Date: Fri, 3 Feb 2017 15:08:35 +0200 Subject: [PATCH 1/3] Fix race condition in JenkinsTriggerHelper --- .../com/offbytwo/jenkins/JenkinsTriggerHelper.java | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/jenkins-client/src/main/java/com/offbytwo/jenkins/JenkinsTriggerHelper.java b/jenkins-client/src/main/java/com/offbytwo/jenkins/JenkinsTriggerHelper.java index 56b0d9a3..e647bc3f 100644 --- a/jenkins-client/src/main/java/com/offbytwo/jenkins/JenkinsTriggerHelper.java +++ b/jenkins-client/src/main/java/com/offbytwo/jenkins/JenkinsTriggerHelper.java @@ -135,16 +135,13 @@ private BuildWithDetails triggerJobAndWaitUntilFinished(String jobName, QueueRef return result; } - job = this.server.getJob(jobName); - Build lastBuild = job.getLastBuild(); - - boolean isBuilding = lastBuild.details().isBuilding(); + Build build = server.getBuild(queueItem); + boolean isBuilding = build.details().isBuilding(); while (isBuilding) { - // TODO: May be we should make this configurable? Thread.sleep(200); - isBuilding = lastBuild.details().isBuilding(); + isBuilding = build.details().isBuilding(); } - return lastBuild.details(); + return build.details(); } } From 984d8284fb01eb406024e57b43d4dced031e0116 Mon Sep 17 00:00:00 2001 From: Karl Heinz Marbaise Date: Fri, 3 Feb 2017 20:40:50 +0100 Subject: [PATCH 2/3] Fixed #220 Followup o getViews() default api/json?depth=1 cause timeout but that change has caused in a failing integration test case which is now fixed by using tree=.. parameters. --- jenkins-client-it-docker/pom.xml | 13 ------------- .../com/offbytwo/jenkins/integration/Constant.java | 6 ++---- .../java/com/offbytwo/jenkins/JenkinsServer.java | 5 +++-- 3 files changed, 5 insertions(+), 19 deletions(-) diff --git a/jenkins-client-it-docker/pom.xml b/jenkins-client-it-docker/pom.xml index 977fb045..72752fee 100644 --- a/jenkins-client-it-docker/pom.xml +++ b/jenkins-client-it-docker/pom.xml @@ -102,20 +102,7 @@ - - http://127.0.0.1:8080 - - - - - env.TRAVIS - - - - http://127.0.0.1:8080/ - - run-docker-its diff --git a/jenkins-client-it-docker/src/test/java/com/offbytwo/jenkins/integration/Constant.java b/jenkins-client-it-docker/src/test/java/com/offbytwo/jenkins/integration/Constant.java index 12961c4e..400a85ac 100644 --- a/jenkins-client-it-docker/src/test/java/com/offbytwo/jenkins/integration/Constant.java +++ b/jenkins-client-it-docker/src/test/java/com/offbytwo/jenkins/integration/Constant.java @@ -6,10 +6,8 @@ public final class Constant { /** * The URL for the running Jenkins server (currently a Docker image). On - * travis it is localhost (127.0.0.1) on my machine it is different. At the - * moment it is solved by a profile in pom..but could that somehow - * identified by docker itself ? + * travis it is localhost (127.0.0.1). */ - public static final URI JENKINS_URI = URI.create(System.getProperty("docker.container.network")); + public static final URI JENKINS_URI = URI.create("http://127.0.0.1:8080/"); } diff --git a/jenkins-client/src/main/java/com/offbytwo/jenkins/JenkinsServer.java b/jenkins-client/src/main/java/com/offbytwo/jenkins/JenkinsServer.java index 0ae72b67..b9147ad3 100644 --- a/jenkins-client/src/main/java/com/offbytwo/jenkins/JenkinsServer.java +++ b/jenkins-client/src/main/java/com/offbytwo/jenkins/JenkinsServer.java @@ -188,11 +188,12 @@ public Map getViews() throws IOException { * @throws IOException in case of an error. */ public Map getViews(FolderJob folder) throws IOException { - List views = client.get(toBaseUrl(folder), MainView.class).getViews(); + // This is much better than using &depth=2 + // http://localhost:8080/api/json?pretty&tree=views[name,url,jobs[name,url]] + List views = client.get(toBaseUrl(folder) + "?tree=views[name,url,jobs[name,url]]", MainView.class).getViews(); return Maps.uniqueIndex(views, new Function() { @Override public String apply(View view) { - view.setClient(client); // TODO: Think about the following? Does there exists a // simpler/more elegant method? From 628dd1a5e344e78bd616756a1da9997894f56524 Mon Sep 17 00:00:00 2001 From: Kaspar Tint Date: Fri, 3 Feb 2017 22:45:25 +0200 Subject: [PATCH 3/3] Simplify the triggerJobAndWaitUntilFinished method a bit Make the polling rate configurable --- .../jenkins/JenkinsTriggerHelper.java | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/jenkins-client/src/main/java/com/offbytwo/jenkins/JenkinsTriggerHelper.java b/jenkins-client/src/main/java/com/offbytwo/jenkins/JenkinsTriggerHelper.java index e647bc3f..3f1ed2b8 100644 --- a/jenkins-client/src/main/java/com/offbytwo/jenkins/JenkinsTriggerHelper.java +++ b/jenkins-client/src/main/java/com/offbytwo/jenkins/JenkinsTriggerHelper.java @@ -19,10 +19,18 @@ */ public class JenkinsTriggerHelper { - private JenkinsServer server; + private final JenkinsServer server; + private final Long retryInterval; + private static final Long DEFAULT_RETRY_INTERVAL = 200L; public JenkinsTriggerHelper(JenkinsServer server) { this.server = server; + this.retryInterval = DEFAULT_RETRY_INTERVAL; + } + + public JenkinsTriggerHelper(JenkinsServer server, Long retryInterval) { + this.server = server; + this.retryInterval = retryInterval; } /** @@ -114,31 +122,23 @@ public BuildWithDetails triggerJobAndWaitUntilFinished(String jobName, boolean c */ private BuildWithDetails triggerJobAndWaitUntilFinished(String jobName, QueueReference queueRef) throws IOException, InterruptedException { - JobWithDetails job; - job = this.server.getJob(jobName); + JobWithDetails job = this.server.getJob(jobName); QueueItem queueItem = this.server.getQueueItem(queueRef); + while (!queueItem.isCancelled() && job.isInQueue()) { - // TODO: May be we should make this configurable? - Thread.sleep(200); + Thread.sleep(retryInterval); job = this.server.getJob(jobName); queueItem = this.server.getQueueItem(queueRef); } + Build build = server.getBuild(queueItem); if (queueItem.isCancelled()) { - // TODO: Check if this is ok? - // We will get the details of the last build. NOT of the cancelled - // build, cause there is no information about that available cause - // it does not exist. - BuildWithDetails result = new BuildWithDetails(job.getLastBuild().details()); - // TODO: Should we add more information here? - result.setResult(BuildResult.CANCELLED); - return result; + return build.details(); } - Build build = server.getBuild(queueItem); boolean isBuilding = build.details().isBuilding(); while (isBuilding) { - Thread.sleep(200); + Thread.sleep(retryInterval); isBuilding = build.details().isBuilding(); }