[BEAM-1376] Move website jobs to main Beam repo to reduce duplication. #1897

jasonkuster · 2017-02-02T00:43:29Z

Signed-off-by: Jason Kuster jasonkuster@google.com

Be sure to do all of the following to help us incorporate your contribution
quickly and easily:

Make sure the PR title is formatted like:
[BEAM-<Jira issue #>] Description of pull request
Make sure tests pass via mvn clean verify. (Even better, enable
Travis-CI on your fork and ensure the whole test matrix passes).
Replace <Jira issue #> in the title with the actual Jira issue
number, if there is one.
If this contribution is large, please file an Apache
Individual Contributor License Agreement.

jasonkuster · 2017-02-02T00:43:36Z

davorbonaci · 2017-02-02T00:44:59Z

.jenkins/common_job_properties.groovy


    // GitHub project.
    context.properties {
-      githubProjectUrl('https://github.com/apache/beam/')
+      githubProjectUrl(repo_link + '/')


I'm not sure it's even necessary to append the '/'. I believe this just sets the url on the job description page.

You are appending .git below, and '/' here. If the given link ends with a '/' neither is correct. One of them is certainly mandatory, so things will fail.

Gave this another shot -- just specifying repo name, which should be less prone to people adding trailing slashes. Thoughts?

Great resolution!

asfbot · 2017-02-02T01:03:37Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/6986/
--none--

asfbot · 2017-02-02T01:06:27Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/6987/
--none--

asfbot · 2017-02-02T01:32:51Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/6989/
--none--

asfbot · 2017-02-02T01:57:09Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/6991/
--none--

davorbonaci

Nice ;-)

davorbonaci · 2017-02-02T05:46:18Z

.jenkins/common_job_properties.groovy

@@ -23,11 +23,12 @@ class common_job_properties {
  // Sets common top-level job properties.
  static def setTopLevelJobProperties(def context,
                                      def default_branch = 'master',
-                                      def default_timeout = 100) {
+                                      def default_timeout = 100,
+                                      def repo_name = 'beam') {


repo_name -> repository_name

davorbonaci · 2017-02-02T05:46:34Z

.jenkins/common_job_properties.groovy

@@ -23,11 +23,12 @@ class common_job_properties {
  // Sets common top-level job properties.
  static def setTopLevelJobProperties(def context,
                                      def default_branch = 'master',


order parameters more naturally: context, repo, branch, timeout.

Params are ordered in terms of likelihood of overriding of parameter, to most effectively use groovy default values. Can reorder, but will introduce unnecessary verbosity into the method calls in all the jobs. Thoughts?

Refactored into methods -- I think it's cleaner this way.

davorbonaci · 2017-02-02T05:48:00Z

.jenkins/job_beam_PostCommit_Website_Test.groovy

@@ -0,0 +1,48 @@
+import common_job_properties


Separate PR?

Sure - will revert in next commit.

davorbonaci · 2017-02-02T05:48:37Z

.jenkins/job_beam_PreCommit_Website_Stage.groovy

@@ -0,0 +1,66 @@
+import common_job_properties


This file could logically stay there too. Is it feasible?

Not feasible without doing ugly things inside the seed job. Seed job builds by cloning this repo and running all jobs inside the .jenkins folder -- we could hypothetically also manually clone the website repo, have its groovy jobs reference (essentially) ../../beam/.jenkins/common... but that seems to me to be a fairly brittle and undesirable solution.

https://git-scm.com/docs/git-submodule?

https://codingkilledthecat.wordpress.com/2012/04/28/why-your-company-shouldnt-use-git-submodules/

I've had bad experiences with submodules relevant to the article I posted there -- if you really want to use them I'll give it a shot, but I think the downsides are way larger than having these two (eventually three) files live in a different repository.

My experience matches too.

Another alternative to consider: another Jenkins step before the DSL step that clones the other repository?

Yeah I'd thought of that but I'm pretty resistant to having the jobs be dependent on the configuration of their environment to succeed -- if something goes wrong with the clone it could adversely affect the jobs running in Jenkins. Having all of our jenkins jobs defined in one place seems like a better choice.

Alright -- stays as is.

davorbonaci · 2017-02-02T05:48:46Z

.jenkins/job_beam_PreCommit_Website_Test.groovy

@@ -0,0 +1,51 @@
+import common_job_properties


Same as above.

jasonkuster · 2017-02-02T20:06:32Z

PTAL: @davorbonaci

asfbot · 2017-02-02T20:09:21Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/7016/
--none--

asfbot · 2017-02-02T20:12:07Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/7017/
--none--

Signed-off-by: Jason Kuster <jasonkuster@google.com>

…operties. Signed-off-by: Jason Kuster <jasonkuster@google.com>

Signed-off-by: Jason Kuster <jasonkuster@google.com>

asfbot · 2017-02-02T21:03:25Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/7026/
--none--

davorbonaci · 2017-02-02T23:15:17Z

.jenkins/common_job_properties.groovy

@@ -155,6 +169,12 @@ class common_job_properties {
    setPullRequestBuildTrigger(context, comment)
  }

+  static def setPreCommitWithSuccessComment(def context,


This doesn't seem necessary -- you have just one default and it is the last parameter.

Sure -- removed.

davorbonaci · 2017-02-02T23:17:24Z

.jenkins/job_seed.groovy

-              'projects based on Jenkins DSL groovy files checked into the ' +
-              'code repository.')
+job('beam_SeedJob') {
+  description('Automatically configures all Apache Beam Jenkins projects based' +


space between "based" and "on"

davorbonaci · 2017-02-02T23:17:54Z

.jenkins/common_job_properties.groovy

-  static def setTopLevelJobProperties(def context,
-                                      def default_branch = 'master',
-                                      def default_timeout = 100) {
+  // Sets common top-level job properties for website repo jobs.


repo -> repository (everywhere)

davorbonaci · 2017-02-02T23:19:19Z

.jenkins/job_beam_PreCommit_Website_Stage.groovy

@@ -0,0 +1,66 @@
+import common_job_properties


https://git-scm.com/docs/git-submodule?

Signed-off-by: Jason Kuster <jasonkuster@google.com>

jasonkuster · 2017-02-02T23:27:12Z

.jenkins/common_job_properties.groovy

@@ -20,14 +20,27 @@
 // common properties that are shared among all Jenkins projects.


This is a continuation of the sentence on the above line -- capitalizing seems wrong here?

jasonkuster · 2017-02-02T23:27:47Z

.jenkins/common_job_properties.groovy

-  static def setTopLevelJobProperties(def context,
-                                      def default_branch = 'master',
-                                      def default_timeout = 100) {
+  // Sets common top-level job properties for website repo jobs.


jasonkuster · 2017-02-02T23:28:47Z

.jenkins/job_seed.groovy

-              'projects based on Jenkins DSL groovy files checked into the ' +
-              'code repository.')
+job('beam_SeedJob') {
+  description('Automatically configures all Apache Beam Jenkins projects based' +


jasonkuster · 2017-02-02T23:36:27Z

.jenkins/job_beam_PreCommit_Website_Stage.groovy

@@ -0,0 +1,66 @@
+import common_job_properties


Yeah I'd thought of that but I'm pretty resistant to having the jobs be dependent on the configuration of their environment to succeed -- if something goes wrong with the clone it could adversely affect the jobs running in Jenkins. Having all of our jenkins jobs defined in one place seems like a better choice.

jasonkuster · 2017-02-02T23:40:40Z

.jenkins/common_job_properties.groovy

@@ -155,6 +169,12 @@ class common_job_properties {
    setPullRequestBuildTrigger(context, comment)
  }

+  static def setPreCommitWithSuccessComment(def context,


Sure -- removed.

asfbot · 2017-02-03T00:18:19Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/7038/
--none--

davorbonaci · 2017-02-03T00:29:52Z

.jenkins/job_beam_PreCommit_Website_Stage.groovy

@@ -0,0 +1,66 @@
+import common_job_properties


Alright -- stays as is.

davorbonaci · 2017-02-03T00:33:37Z

.jenkins/common_job_properties.groovy

@@ -150,9 +164,11 @@ class common_job_properties {
  }

  // Sets common config for PreCommit jobs.
-  static def setPreCommit(def context, comment) {
+  static def setPreCommit(def context,
+                          def comment,


comment is very ambiguous. commitStatusName might be better.

Signed-off-by: Jason Kuster <jasonkuster@google.com>

davorbonaci

LGTM. Merging.

asfbot · 2017-02-03T01:10:10Z

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/beam_PreCommit_Java_MavenInstall/7044/
--none--

davorbonaci requested changes Feb 2, 2017

View reviewed changes

jasonkuster mentioned this pull request Feb 2, 2017

[BEAM-1376] Delete jobs due to move to main repo. apache/beam-site#143

Closed

davorbonaci requested changes Feb 2, 2017

View reviewed changes

jasonkuster added 6 commits February 2, 2017 12:45

Move website jobs to main Beam repo to reduce duplication.

d87b1dd

Signed-off-by: Jason Kuster <jasonkuster@google.com>

add tool used by jobs as well.

b31f8cd

Signed-off-by: Jason Kuster <jasonkuster@google.com>

Make repo specification less brittle.

1a96085

Signed-off-by: Jason Kuster <jasonkuster@google.com>

Tool should not live in main beam repo -- deleting.

0699dcc

Signed-off-by: Jason Kuster <jasonkuster@google.com>

Refactored repo details and some precommit details into common_job_pr…

759455c

…operties. Signed-off-by: Jason Kuster <jasonkuster@google.com>

Name fixup

3c383fe

Signed-off-by: Jason Kuster <jasonkuster@google.com>

jasonkuster force-pushed the website-jobs branch from 011427e to 3c383fe Compare February 2, 2017 20:52

davorbonaci requested changes Feb 2, 2017

View reviewed changes

Address PR comments.

47cca8d

Signed-off-by: Jason Kuster <jasonkuster@google.com>

jasonkuster commented Feb 2, 2017

View reviewed changes

jasonkuster mentioned this pull request Feb 2, 2017

[BEAM-1374] Enable oneshots of Jenkins runs in precommit. #1892

Closed

4 tasks

davorbonaci approved these changes Feb 3, 2017

View reviewed changes

Nit fix.

e30a4c0

Signed-off-by: Jason Kuster <jasonkuster@google.com>

davorbonaci approved these changes Feb 3, 2017

View reviewed changes

asfgit closed this in 1533d70 Feb 3, 2017

jasonkuster deleted the website-jobs branch February 3, 2017 01:19

		@@ -20,14 +20,27 @@
		// common properties that are shared among all Jenkins projects.

[BEAM-1376] Move website jobs to main Beam repo to reduce duplication. #1897

[BEAM-1376] Move website jobs to main Beam repo to reduce duplication. #1897

Conversation

jasonkuster commented Feb 2, 2017

jasonkuster commented Feb 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asfbot commented Feb 2, 2017

asfbot commented Feb 2, 2017

asfbot commented Feb 2, 2017

asfbot commented Feb 2, 2017

davorbonaci left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasonkuster commented Feb 2, 2017

asfbot commented Feb 2, 2017

asfbot commented Feb 2, 2017

asfbot commented Feb 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asfbot commented Feb 3, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davorbonaci left a comment

Choose a reason for hiding this comment

asfbot commented Feb 3, 2017