You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2020/06/19 21:17:07 UTC

[GitHub] [beam] JustineKoa opened a new pull request #12042: WIP: Jenkins Plugin

JustineKoa opened a new pull request #12042:
URL: https://github.com/apache/beam/pull/12042


   Currently able to get configurations from user: Java or Python, Gradle (show added build task) or Maven
   
   Working on:
   
   - testing python, maven, gradle commands work
   - getting google cloud credentials
   - navigating to the correct directory to execute the commands
   
   ------------------------
   
   Post-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   Lang | SDK | Apex | Dataflow | Flink | Samza | Spark
   --- | --- | --- | --- | --- | --- | ---
   Go | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go/lastCompletedBuild/) | --- | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Go_VR_Spark/lastCompletedBuild/)
   Java | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Apex/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Dataflow_Java11/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Flink_Java11/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Batch/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Flink_Streaming/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Samza/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_Spark/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_PVR_Spark_Batch/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Java_ValidatesRunner_SparkStructuredStreaming/lastCompletedBuild/)
   Python | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python2/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python35/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python36/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python37/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_VR_Dataflow_V2/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Py_ValCont/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Python2_PVR_Flink_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python35_VR_Flink/lastCompletedBuild/) | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_Python_VR_Spark/lastCompletedBuild/)
   XLang | --- | --- | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Flink/lastCompletedBuild/) | --- | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PostCommit_XVR_Spark/lastCompletedBuild/)
   
   Pre-Commit Tests Status (on master branch)
   ------------------------------------------------------------------------------------------------
   
   --- |Java | Python | Go | Website
   --- | --- | --- | --- | ---
   Non-portable | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Java_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Python_Cron/lastCompletedBuild/)<br>[![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_PythonLint_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Go_Cron/lastCompletedBuild/) | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Website_Cron/lastCompletedBuild/)
   Portable | --- | [![Build Status](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/badge/icon)](https://ci-beam.apache.org/job/beam_PreCommit_Portable_Python_Cron/lastCompletedBuild/) | --- | ---
   
   See [.test-infra/jenkins/README](https://github.com/apache/beam/blob/master/.test-infra/jenkins/README.md) for trigger phrase, status and link of all Jenkins jobs.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] angoenka commented on a change in pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
angoenka commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r448215789



##########
File path: beam-ci/src/main/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.jenkins.plugins;
+
+import hudson.Launcher;
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.util.FormValidation;
+import hudson.model.AbstractProject;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.tasks.Builder;
+import hudson.tasks.BuildStepDescriptor;
+import jenkins.util.SystemProperties;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
+
+import javax.servlet.ServletException;
+import java.io.*;
+import java.util.*;
+
+import jenkins.tasks.SimpleBuildStep;
+
+public class ExecuteBeamPipelineOnDataflowBuilder extends Builder implements SimpleBuildStep {
+
+    private ProcessBuilder processBuilder;
+    private final String pathToCreds;
+    private final String pathToMainClass;
+    private final String pipelineOptions;
+    private final String buildReleaseOptions;
+    private boolean useJava; // if false, use Python
+    private boolean useGradle; // if false, use Maven
+    private ArrayList<String> command;
+    private boolean test; // if we are testing, the mock ProcessBuilder never starts
+
+    @DataBoundConstructor
+    public ExecuteBeamPipelineOnDataflowBuilder(String pathToCreds, String pathToMainClass, String pipelineOptions, String buildReleaseOptions, boolean useJava, boolean useGradle) {
+        this.pathToCreds = pathToCreds;
+        this.pathToMainClass = pathToMainClass;
+        this.pipelineOptions = pipelineOptions;
+        this.buildReleaseOptions = buildReleaseOptions;
+        this.useJava = useJava;
+        this.useGradle = useGradle;
+        this.test = false;
+    }
+
+    public void setProcessBuilder(ProcessBuilder processBuilder) {
+        this.processBuilder = processBuilder;
+        this.test = true;
+    }
+
+    public String getPathToCreds() { return pathToCreds; }
+
+    public String getPathToMainClass() {
+        return pathToMainClass;
+    }
+
+    public String getPipelineOptions() {
+        return pipelineOptions;
+    }
+
+    public String getBuildReleaseOptions() {
+        return buildReleaseOptions;
+    }
+
+    public boolean getUseJava() {
+        return useJava;
+    }
+
+    public boolean getUseGradle() {
+        return useGradle;
+    }
+
+    public ArrayList<String> getCommand() { return command; }
+
+    /**
+     * Builds and sets the command on the ProcessBuilder depending on configurations set by the user
+     * */
+    private void buildCommand(Run<?, ?> run, String workspace, PrintStream logger) {
+//        ArrayList<String> command;
+        if (this.useJava) {
+            String pipelineOptions = this.pipelineOptions.replaceAll("[\\t\\n]+"," ");
+            if (this.useGradle) { // gradle
+                command = new ArrayList<>(Arrays.asList("gradle", "clean", "execute", "-DmainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            } else { // maven
+                command = new ArrayList<>(Arrays.asList("mvn", "compile", "exec:java", "-Dexec.mainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            }
+
+            // add pipeline and build release options if included
+            if (!this.buildReleaseOptions.equals("")) {
+                String[] buildReleaseOptions = this.buildReleaseOptions.split("\\s+"); // split build release options by whitespace
+                command.addAll(Arrays.asList(buildReleaseOptions)); // add build release options as separate list elements
+            }
+        } else { // python
+            // Get Path to the Bash Script
+            String dir = System.getProperty("user.dir"); // Get the directory the plugin is located
+            String pathToScript = dir + "/src/main/java/io/jenkins/plugins/executePythonBeamPipeline.sh";
+
+            // Get Path to the current build directory to create Virtual Environment in
+            String jobBuildPathDirectory = getJobBuildDirectory(run);
+
+            // Execute Bash Script
+            command = new ArrayList<>(Arrays.asList(pathToScript, workspace, jobBuildPathDirectory, this.pathToMainClass, this.pipelineOptions, this.buildReleaseOptions));
+        }
+        this.processBuilder.command(command);
+    }
+
+    /**
+     * @return absolute path to current build folder
+     * */
+    private String getJobBuildDirectory(Run<?, ?> run) {
+        String jenkinsHome = System.getProperty("JENKINS_HOME");
+        String jobName = run.getFullDisplayName().split(" ")[0];
+        int buildNumber = run.getNumber();
+        return jenkinsHome + "/jobs/" + jobName + "/builds/" + buildNumber;
+    }
+
+    @Override
+    public void perform(Run<?, ?> run, FilePath workspace, Launcher launcher, TaskListener listener) throws InterruptedException, IOException {
+//        ProcessBuilder processBuilder = new ProcessBuilder();

Review comment:
       Shall we remove this line.
   
   Also we can create another overloaded method perform which takes ProcessBuilder as an argument instead which you can use in the test.
   
   This perform method can simply call that perform method.
   
   Also we can get rid of exposing setProcessBuilder.

##########
File path: beam-ci/src/main/resources/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder/config.jelly
##########
@@ -0,0 +1,36 @@
+<?jelly escape-by-default='true'?>
+<j:jelly xmlns:j="jelly:core" xmlns:g="glide" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:g2="null" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
+    <p>Make sure you complete the setup steps at <a href="https://beam.apache.org/documentation/runners/dataflow/#setup" target="_blank">https://beam.apache.org/documentation/runners/dataflow/#setup</a></p>
+    <f:entry title="Path to Google Application Credentials" field="pathToCreds">
+        <f:textbox />
+    </f:entry>
+    <f:entry title="Path to Main Class" field="pathToMainClass">
+        <f:textbox />
+    </f:entry>
+    <f:entry title="Pipeline Options" field="pipelineOptions">
+        <f:textarea />
+    </f:entry>
+    <f:entry title="[Optional] Build Release Options" field="buildReleaseOptions">
+        <f:textbox />
+    </f:entry>
+    <f:block>
+      <table>
+        <f:optionalBlock inline="true" name="useJava" title="Check if using Java, leave unchecked if using Python." field="useJava">

Review comment:
       Can we convert them to a drop down.
   When Java is selected we can show 1 more drop down for Gradle/Maven

##########
File path: beam-ci/src/test/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilderTest.java
##########
@@ -0,0 +1,103 @@
+package io.jenkins.plugins;

Review comment:
       Lets add a license header here.

##########
File path: beam-ci/src/main/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.jenkins.plugins;
+
+import hudson.Launcher;
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.util.FormValidation;
+import hudson.model.AbstractProject;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.tasks.Builder;
+import hudson.tasks.BuildStepDescriptor;
+import jenkins.util.SystemProperties;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
+
+import javax.servlet.ServletException;
+import java.io.*;
+import java.util.*;
+
+import jenkins.tasks.SimpleBuildStep;
+
+public class ExecuteBeamPipelineOnDataflowBuilder extends Builder implements SimpleBuildStep {
+
+    private ProcessBuilder processBuilder;
+    private final String pathToCreds;
+    private final String pathToMainClass;
+    private final String pipelineOptions;
+    private final String buildReleaseOptions;
+    private boolean useJava; // if false, use Python
+    private boolean useGradle; // if false, use Maven
+    private ArrayList<String> command;
+    private boolean test; // if we are testing, the mock ProcessBuilder never starts
+
+    @DataBoundConstructor
+    public ExecuteBeamPipelineOnDataflowBuilder(String pathToCreds, String pathToMainClass, String pipelineOptions, String buildReleaseOptions, boolean useJava, boolean useGradle) {
+        this.pathToCreds = pathToCreds;
+        this.pathToMainClass = pathToMainClass;
+        this.pipelineOptions = pipelineOptions;
+        this.buildReleaseOptions = buildReleaseOptions;
+        this.useJava = useJava;
+        this.useGradle = useGradle;
+        this.test = false;
+    }
+
+    public void setProcessBuilder(ProcessBuilder processBuilder) {

Review comment:
       Let's use default access modifier here as I think this is only used in testing.

##########
File path: beam-ci/src/main/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.jenkins.plugins;
+
+import hudson.Launcher;
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.util.FormValidation;
+import hudson.model.AbstractProject;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.tasks.Builder;
+import hudson.tasks.BuildStepDescriptor;
+import jenkins.util.SystemProperties;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
+
+import javax.servlet.ServletException;
+import java.io.*;
+import java.util.*;
+
+import jenkins.tasks.SimpleBuildStep;
+
+public class ExecuteBeamPipelineOnDataflowBuilder extends Builder implements SimpleBuildStep {
+
+    private ProcessBuilder processBuilder;
+    private final String pathToCreds;
+    private final String pathToMainClass;
+    private final String pipelineOptions;
+    private final String buildReleaseOptions;
+    private boolean useJava; // if false, use Python
+    private boolean useGradle; // if false, use Maven
+    private ArrayList<String> command;
+    private boolean test; // if we are testing, the mock ProcessBuilder never starts
+
+    @DataBoundConstructor
+    public ExecuteBeamPipelineOnDataflowBuilder(String pathToCreds, String pathToMainClass, String pipelineOptions, String buildReleaseOptions, boolean useJava, boolean useGradle) {
+        this.pathToCreds = pathToCreds;
+        this.pathToMainClass = pathToMainClass;
+        this.pipelineOptions = pipelineOptions;
+        this.buildReleaseOptions = buildReleaseOptions;
+        this.useJava = useJava;
+        this.useGradle = useGradle;
+        this.test = false;
+    }
+
+    public void setProcessBuilder(ProcessBuilder processBuilder) {
+        this.processBuilder = processBuilder;
+        this.test = true;
+    }
+
+    public String getPathToCreds() { return pathToCreds; }
+
+    public String getPathToMainClass() {
+        return pathToMainClass;
+    }
+
+    public String getPipelineOptions() {
+        return pipelineOptions;
+    }
+
+    public String getBuildReleaseOptions() {
+        return buildReleaseOptions;
+    }
+
+    public boolean getUseJava() {
+        return useJava;
+    }
+
+    public boolean getUseGradle() {
+        return useGradle;
+    }
+
+    public ArrayList<String> getCommand() { return command; }
+
+    /**
+     * Builds and sets the command on the ProcessBuilder depending on configurations set by the user
+     * */
+    private void buildCommand(Run<?, ?> run, String workspace, PrintStream logger) {
+//        ArrayList<String> command;
+        if (this.useJava) {
+            String pipelineOptions = this.pipelineOptions.replaceAll("[\\t\\n]+"," ");
+            if (this.useGradle) { // gradle
+                command = new ArrayList<>(Arrays.asList("gradle", "clean", "execute", "-DmainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            } else { // maven
+                command = new ArrayList<>(Arrays.asList("mvn", "compile", "exec:java", "-Dexec.mainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            }
+
+            // add pipeline and build release options if included
+            if (!this.buildReleaseOptions.equals("")) {
+                String[] buildReleaseOptions = this.buildReleaseOptions.split("\\s+"); // split build release options by whitespace
+                command.addAll(Arrays.asList(buildReleaseOptions)); // add build release options as separate list elements
+            }
+        } else { // python
+            // Get Path to the Bash Script
+            String dir = System.getProperty("user.dir"); // Get the directory the plugin is located
+            String pathToScript = dir + "/src/main/java/io/jenkins/plugins/executePythonBeamPipeline.sh";
+
+            // Get Path to the current build directory to create Virtual Environment in
+            String jobBuildPathDirectory = getJobBuildDirectory(run);
+
+            // Execute Bash Script
+            command = new ArrayList<>(Arrays.asList(pathToScript, workspace, jobBuildPathDirectory, this.pathToMainClass, this.pipelineOptions, this.buildReleaseOptions));
+        }
+        this.processBuilder.command(command);
+    }
+
+    /**
+     * @return absolute path to current build folder
+     * */
+    private String getJobBuildDirectory(Run<?, ?> run) {
+        String jenkinsHome = System.getProperty("JENKINS_HOME");
+        String jobName = run.getFullDisplayName().split(" ")[0];
+        int buildNumber = run.getNumber();
+        return jenkinsHome + "/jobs/" + jobName + "/builds/" + buildNumber;
+    }
+
+    @Override
+    public void perform(Run<?, ?> run, FilePath workspace, Launcher launcher, TaskListener listener) throws InterruptedException, IOException {
+//        ProcessBuilder processBuilder = new ProcessBuilder();
+        if (processBuilder == null) {
+            this.processBuilder = new ProcessBuilder();
+        }
+
+        // see that all configurations are received correctly
+        listener.getLogger().println("path to google cloud credentials : " + this.pathToCreds);
+        listener.getLogger().println("path to main class : " + this.pathToMainClass);
+        listener.getLogger().println("pipeline options : " + this.pipelineOptions);
+        listener.getLogger().println("build release options : " + this.buildReleaseOptions);
+        listener.getLogger().println("use java: " + this.useJava);
+        listener.getLogger().println("use gradle: " + this.useGradle);
+
+        Map<String, String> env = this.processBuilder.environment();
+        env.put("GOOGLE_APPLICATION_CREDENTIALS", this.pathToCreds);
+
+        // set correct directory to be running command
+        this.processBuilder.directory(new File(workspace.toURI()));
+
+        // build and set command to processBuilder based on configurations
+        buildCommand(run, workspace.toURI().getPath(), listener.getLogger());
+
+        if (this.test) { // if we are testing commands only, return before starting process

Review comment:
       You can mock "start" method to return a mock process then you will not need additional `test` flag.
   
   In general, it's good to avoid adding any code for test in production/main code.

##########
File path: beam-ci/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker
##########
@@ -0,0 +1 @@
+mock-maker-inline

Review comment:
       Why do we need this extension?

##########
File path: beam-ci/src/main/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.jenkins.plugins;
+
+import hudson.Launcher;
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.util.FormValidation;
+import hudson.model.AbstractProject;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.tasks.Builder;
+import hudson.tasks.BuildStepDescriptor;
+import jenkins.util.SystemProperties;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
+
+import javax.servlet.ServletException;
+import java.io.*;
+import java.util.*;
+
+import jenkins.tasks.SimpleBuildStep;
+
+public class ExecuteBeamPipelineOnDataflowBuilder extends Builder implements SimpleBuildStep {
+
+    private ProcessBuilder processBuilder;
+    private final String pathToCreds;
+    private final String pathToMainClass;
+    private final String pipelineOptions;
+    private final String buildReleaseOptions;
+    private boolean useJava; // if false, use Python
+    private boolean useGradle; // if false, use Maven
+    private ArrayList<String> command;
+    private boolean test; // if we are testing, the mock ProcessBuilder never starts
+
+    @DataBoundConstructor
+    public ExecuteBeamPipelineOnDataflowBuilder(String pathToCreds, String pathToMainClass, String pipelineOptions, String buildReleaseOptions, boolean useJava, boolean useGradle) {
+        this.pathToCreds = pathToCreds;
+        this.pathToMainClass = pathToMainClass;
+        this.pipelineOptions = pipelineOptions;
+        this.buildReleaseOptions = buildReleaseOptions;
+        this.useJava = useJava;
+        this.useGradle = useGradle;
+        this.test = false;
+    }
+
+    public void setProcessBuilder(ProcessBuilder processBuilder) {
+        this.processBuilder = processBuilder;
+        this.test = true;
+    }
+
+    public String getPathToCreds() { return pathToCreds; }
+
+    public String getPathToMainClass() {
+        return pathToMainClass;
+    }
+
+    public String getPipelineOptions() {
+        return pipelineOptions;
+    }
+
+    public String getBuildReleaseOptions() {
+        return buildReleaseOptions;
+    }
+
+    public boolean getUseJava() {
+        return useJava;
+    }
+
+    public boolean getUseGradle() {
+        return useGradle;
+    }
+
+    public ArrayList<String> getCommand() { return command; }
+
+    /**
+     * Builds and sets the command on the ProcessBuilder depending on configurations set by the user
+     * */
+    private void buildCommand(Run<?, ?> run, String workspace, PrintStream logger) {
+//        ArrayList<String> command;
+        if (this.useJava) {
+            String pipelineOptions = this.pipelineOptions.replaceAll("[\\t\\n]+"," ");
+            if (this.useGradle) { // gradle
+                command = new ArrayList<>(Arrays.asList("gradle", "clean", "execute", "-DmainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            } else { // maven
+                command = new ArrayList<>(Arrays.asList("mvn", "compile", "exec:java", "-Dexec.mainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            }
+
+            // add pipeline and build release options if included
+            if (!this.buildReleaseOptions.equals("")) {
+                String[] buildReleaseOptions = this.buildReleaseOptions.split("\\s+"); // split build release options by whitespace
+                command.addAll(Arrays.asList(buildReleaseOptions)); // add build release options as separate list elements
+            }
+        } else { // python
+            // Get Path to the Bash Script
+            String dir = System.getProperty("user.dir"); // Get the directory the plugin is located
+            String pathToScript = dir + "/src/main/java/io/jenkins/plugins/executePythonBeamPipeline.sh";
+
+            // Get Path to the current build directory to create Virtual Environment in
+            String jobBuildPathDirectory = getJobBuildDirectory(run);
+
+            // Execute Bash Script
+            command = new ArrayList<>(Arrays.asList(pathToScript, workspace, jobBuildPathDirectory, this.pathToMainClass, this.pipelineOptions, this.buildReleaseOptions));
+        }
+        this.processBuilder.command(command);
+    }
+
+    /**
+     * @return absolute path to current build folder
+     * */
+    private String getJobBuildDirectory(Run<?, ?> run) {
+        String jenkinsHome = System.getProperty("JENKINS_HOME");
+        String jobName = run.getFullDisplayName().split(" ")[0];
+        int buildNumber = run.getNumber();
+        return jenkinsHome + "/jobs/" + jobName + "/builds/" + buildNumber;
+    }
+
+    @Override
+    public void perform(Run<?, ?> run, FilePath workspace, Launcher launcher, TaskListener listener) throws InterruptedException, IOException {
+//        ProcessBuilder processBuilder = new ProcessBuilder();
+        if (processBuilder == null) {
+            this.processBuilder = new ProcessBuilder();
+        }
+
+        // see that all configurations are received correctly
+        listener.getLogger().println("path to google cloud credentials : " + this.pathToCreds);
+        listener.getLogger().println("path to main class : " + this.pathToMainClass);
+        listener.getLogger().println("pipeline options : " + this.pipelineOptions);
+        listener.getLogger().println("build release options : " + this.buildReleaseOptions);
+        listener.getLogger().println("use java: " + this.useJava);
+        listener.getLogger().println("use gradle: " + this.useGradle);
+
+        Map<String, String> env = this.processBuilder.environment();
+        env.put("GOOGLE_APPLICATION_CREDENTIALS", this.pathToCreds);
+
+        // set correct directory to be running command
+        this.processBuilder.directory(new File(workspace.toURI()));
+
+        // build and set command to processBuilder based on configurations
+        buildCommand(run, workspace.toURI().getPath(), listener.getLogger());
+
+        if (this.test) { // if we are testing commands only, return before starting process
+            listener.getLogger().println("Test finished successfully.");
+            return;
+        }
+
+        Process process = this.processBuilder.start();

Review comment:
       We can also log complete processBuilder command here.

##########
File path: beam-ci/src/main/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.jenkins.plugins;
+
+import hudson.Launcher;
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.util.FormValidation;
+import hudson.model.AbstractProject;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.tasks.Builder;
+import hudson.tasks.BuildStepDescriptor;
+import jenkins.util.SystemProperties;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
+
+import javax.servlet.ServletException;
+import java.io.*;
+import java.util.*;
+
+import jenkins.tasks.SimpleBuildStep;
+
+public class ExecuteBeamPipelineOnDataflowBuilder extends Builder implements SimpleBuildStep {
+
+    private ProcessBuilder processBuilder;
+    private final String pathToCreds;
+    private final String pathToMainClass;
+    private final String pipelineOptions;
+    private final String buildReleaseOptions;
+    private boolean useJava; // if false, use Python
+    private boolean useGradle; // if false, use Maven
+    private ArrayList<String> command;
+    private boolean test; // if we are testing, the mock ProcessBuilder never starts
+
+    @DataBoundConstructor
+    public ExecuteBeamPipelineOnDataflowBuilder(String pathToCreds, String pathToMainClass, String pipelineOptions, String buildReleaseOptions, boolean useJava, boolean useGradle) {
+        this.pathToCreds = pathToCreds;
+        this.pathToMainClass = pathToMainClass;
+        this.pipelineOptions = pipelineOptions;
+        this.buildReleaseOptions = buildReleaseOptions;
+        this.useJava = useJava;
+        this.useGradle = useGradle;
+        this.test = false;
+    }
+
+    public void setProcessBuilder(ProcessBuilder processBuilder) {
+        this.processBuilder = processBuilder;
+        this.test = true;
+    }
+
+    public String getPathToCreds() { return pathToCreds; }
+
+    public String getPathToMainClass() {
+        return pathToMainClass;
+    }
+
+    public String getPipelineOptions() {
+        return pipelineOptions;
+    }
+
+    public String getBuildReleaseOptions() {
+        return buildReleaseOptions;
+    }
+
+    public boolean getUseJava() {
+        return useJava;
+    }
+
+    public boolean getUseGradle() {
+        return useGradle;
+    }
+
+    public ArrayList<String> getCommand() { return command; }
+
+    /**
+     * Builds and sets the command on the ProcessBuilder depending on configurations set by the user
+     * */
+    private void buildCommand(Run<?, ?> run, String workspace, PrintStream logger) {
+//        ArrayList<String> command;
+        if (this.useJava) {
+            String pipelineOptions = this.pipelineOptions.replaceAll("[\\t\\n]+"," ");
+            if (this.useGradle) { // gradle
+                command = new ArrayList<>(Arrays.asList("gradle", "clean", "execute", "-DmainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            } else { // maven
+                command = new ArrayList<>(Arrays.asList("mvn", "compile", "exec:java", "-Dexec.mainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            }
+
+            // add pipeline and build release options if included
+            if (!this.buildReleaseOptions.equals("")) {
+                String[] buildReleaseOptions = this.buildReleaseOptions.split("\\s+"); // split build release options by whitespace
+                command.addAll(Arrays.asList(buildReleaseOptions)); // add build release options as separate list elements
+            }
+        } else { // python
+            // Get Path to the Bash Script
+            String dir = System.getProperty("user.dir"); // Get the directory the plugin is located
+            String pathToScript = dir + "/src/main/java/io/jenkins/plugins/executePythonBeamPipeline.sh";

Review comment:
       Let's make the filename a constant and define it at the top.

##########
File path: beam-ci/src/main/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.jenkins.plugins;
+
+import hudson.Launcher;
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.util.FormValidation;
+import hudson.model.AbstractProject;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.tasks.Builder;
+import hudson.tasks.BuildStepDescriptor;
+import jenkins.util.SystemProperties;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
+
+import javax.servlet.ServletException;
+import java.io.*;
+import java.util.*;
+
+import jenkins.tasks.SimpleBuildStep;
+
+public class ExecuteBeamPipelineOnDataflowBuilder extends Builder implements SimpleBuildStep {
+
+    private ProcessBuilder processBuilder;
+    private final String pathToCreds;
+    private final String pathToMainClass;
+    private final String pipelineOptions;
+    private final String buildReleaseOptions;
+    private boolean useJava; // if false, use Python
+    private boolean useGradle; // if false, use Maven
+    private ArrayList<String> command;
+    private boolean test; // if we are testing, the mock ProcessBuilder never starts
+
+    @DataBoundConstructor
+    public ExecuteBeamPipelineOnDataflowBuilder(String pathToCreds, String pathToMainClass, String pipelineOptions, String buildReleaseOptions, boolean useJava, boolean useGradle) {

Review comment:
       Let's make useJava -> language(Enum/String) to support more languages.
   Let's make useGradle -> buildSystem(Enum/String) to support more build systems
   
   
   Only try to do the next part if it's easy. Don't spend too much time in this part ->
   Let's make pipelineOptions a list of strings for easier parsing of options.
   Let's make buildReleaseOptions a list of strings for easier parsing.
   
   Correspondingly, we will have to change the UI to take a list of parameters of possible. 

##########
File path: beam-ci/src/main/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.jenkins.plugins;
+
+import hudson.Launcher;
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.util.FormValidation;
+import hudson.model.AbstractProject;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.tasks.Builder;
+import hudson.tasks.BuildStepDescriptor;
+import jenkins.util.SystemProperties;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
+
+import javax.servlet.ServletException;
+import java.io.*;
+import java.util.*;
+
+import jenkins.tasks.SimpleBuildStep;
+
+public class ExecuteBeamPipelineOnDataflowBuilder extends Builder implements SimpleBuildStep {
+
+    private ProcessBuilder processBuilder;
+    private final String pathToCreds;
+    private final String pathToMainClass;
+    private final String pipelineOptions;
+    private final String buildReleaseOptions;
+    private boolean useJava; // if false, use Python
+    private boolean useGradle; // if false, use Maven
+    private ArrayList<String> command;
+    private boolean test; // if we are testing, the mock ProcessBuilder never starts
+
+    @DataBoundConstructor
+    public ExecuteBeamPipelineOnDataflowBuilder(String pathToCreds, String pathToMainClass, String pipelineOptions, String buildReleaseOptions, boolean useJava, boolean useGradle) {
+        this.pathToCreds = pathToCreds;
+        this.pathToMainClass = pathToMainClass;
+        this.pipelineOptions = pipelineOptions;
+        this.buildReleaseOptions = buildReleaseOptions;
+        this.useJava = useJava;
+        this.useGradle = useGradle;
+        this.test = false;
+    }
+
+    public void setProcessBuilder(ProcessBuilder processBuilder) {
+        this.processBuilder = processBuilder;
+        this.test = true;
+    }
+
+    public String getPathToCreds() { return pathToCreds; }
+
+    public String getPathToMainClass() {
+        return pathToMainClass;
+    }
+
+    public String getPipelineOptions() {
+        return pipelineOptions;
+    }
+
+    public String getBuildReleaseOptions() {
+        return buildReleaseOptions;
+    }
+
+    public boolean getUseJava() {
+        return useJava;
+    }
+
+    public boolean getUseGradle() {
+        return useGradle;
+    }
+
+    public ArrayList<String> getCommand() { return command; }
+
+    /**
+     * Builds and sets the command on the ProcessBuilder depending on configurations set by the user
+     * */
+    private void buildCommand(Run<?, ?> run, String workspace, PrintStream logger) {
+//        ArrayList<String> command;

Review comment:
       Shall we remove this line.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] JustineKoa commented on a change in pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
JustineKoa commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r448536674



##########
File path: beam-ci/src/main/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.jenkins.plugins;
+
+import hudson.Launcher;
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.util.FormValidation;
+import hudson.model.AbstractProject;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.tasks.Builder;
+import hudson.tasks.BuildStepDescriptor;
+import jenkins.util.SystemProperties;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
+
+import javax.servlet.ServletException;
+import java.io.*;
+import java.util.*;
+
+import jenkins.tasks.SimpleBuildStep;
+
+public class ExecuteBeamPipelineOnDataflowBuilder extends Builder implements SimpleBuildStep {
+
+    private ProcessBuilder processBuilder;
+    private final String pathToCreds;
+    private final String pathToMainClass;
+    private final String pipelineOptions;
+    private final String buildReleaseOptions;
+    private boolean useJava; // if false, use Python
+    private boolean useGradle; // if false, use Maven
+    private ArrayList<String> command;
+    private boolean test; // if we are testing, the mock ProcessBuilder never starts
+
+    @DataBoundConstructor
+    public ExecuteBeamPipelineOnDataflowBuilder(String pathToCreds, String pathToMainClass, String pipelineOptions, String buildReleaseOptions, boolean useJava, boolean useGradle) {
+        this.pathToCreds = pathToCreds;
+        this.pathToMainClass = pathToMainClass;
+        this.pipelineOptions = pipelineOptions;
+        this.buildReleaseOptions = buildReleaseOptions;
+        this.useJava = useJava;
+        this.useGradle = useGradle;
+        this.test = false;
+    }
+
+    public void setProcessBuilder(ProcessBuilder processBuilder) {
+        this.processBuilder = processBuilder;
+        this.test = true;
+    }
+
+    public String getPathToCreds() { return pathToCreds; }
+
+    public String getPathToMainClass() {
+        return pathToMainClass;
+    }
+
+    public String getPipelineOptions() {
+        return pipelineOptions;
+    }
+
+    public String getBuildReleaseOptions() {
+        return buildReleaseOptions;
+    }
+
+    public boolean getUseJava() {
+        return useJava;
+    }
+
+    public boolean getUseGradle() {
+        return useGradle;
+    }
+
+    public ArrayList<String> getCommand() { return command; }
+
+    /**
+     * Builds and sets the command on the ProcessBuilder depending on configurations set by the user
+     * */
+    private void buildCommand(Run<?, ?> run, String workspace, PrintStream logger) {
+//        ArrayList<String> command;
+        if (this.useJava) {
+            String pipelineOptions = this.pipelineOptions.replaceAll("[\\t\\n]+"," ");
+            if (this.useGradle) { // gradle
+                command = new ArrayList<>(Arrays.asList("gradle", "clean", "execute", "-DmainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            } else { // maven
+                command = new ArrayList<>(Arrays.asList("mvn", "compile", "exec:java", "-Dexec.mainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            }
+
+            // add pipeline and build release options if included
+            if (!this.buildReleaseOptions.equals("")) {
+                String[] buildReleaseOptions = this.buildReleaseOptions.split("\\s+"); // split build release options by whitespace
+                command.addAll(Arrays.asList(buildReleaseOptions)); // add build release options as separate list elements
+            }
+        } else { // python
+            // Get Path to the Bash Script
+            String dir = System.getProperty("user.dir"); // Get the directory the plugin is located
+            String pathToScript = dir + "/src/main/java/io/jenkins/plugins/executePythonBeamPipeline.sh";
+
+            // Get Path to the current build directory to create Virtual Environment in
+            String jobBuildPathDirectory = getJobBuildDirectory(run);
+
+            // Execute Bash Script
+            command = new ArrayList<>(Arrays.asList(pathToScript, workspace, jobBuildPathDirectory, this.pathToMainClass, this.pipelineOptions, this.buildReleaseOptions));
+        }
+        this.processBuilder.command(command);
+    }
+
+    /**
+     * @return absolute path to current build folder
+     * */
+    private String getJobBuildDirectory(Run<?, ?> run) {
+        String jenkinsHome = System.getProperty("JENKINS_HOME");
+        String jobName = run.getFullDisplayName().split(" ")[0];
+        int buildNumber = run.getNumber();
+        return jenkinsHome + "/jobs/" + jobName + "/builds/" + buildNumber;
+    }
+
+    @Override
+    public void perform(Run<?, ?> run, FilePath workspace, Launcher launcher, TaskListener listener) throws InterruptedException, IOException {
+//        ProcessBuilder processBuilder = new ProcessBuilder();
+        if (processBuilder == null) {
+            this.processBuilder = new ProcessBuilder();
+        }
+
+        // see that all configurations are received correctly
+        listener.getLogger().println("path to google cloud credentials : " + this.pathToCreds);
+        listener.getLogger().println("path to main class : " + this.pathToMainClass);
+        listener.getLogger().println("pipeline options : " + this.pipelineOptions);
+        listener.getLogger().println("build release options : " + this.buildReleaseOptions);
+        listener.getLogger().println("use java: " + this.useJava);
+        listener.getLogger().println("use gradle: " + this.useGradle);
+
+        Map<String, String> env = this.processBuilder.environment();
+        env.put("GOOGLE_APPLICATION_CREDENTIALS", this.pathToCreds);
+
+        // set correct directory to be running command
+        this.processBuilder.directory(new File(workspace.toURI()));
+
+        // build and set command to processBuilder based on configurations
+        buildCommand(run, workspace.toURI().getPath(), listener.getLogger());
+
+        if (this.test) { // if we are testing commands only, return before starting process

Review comment:
       The reason I'm asking/a bit confused is because I create BufferedReader depending on the process InputStream and ErrorStream, so I'm not sure how I can set it to be a mock in the tests.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] JustineKoa commented on a change in pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
JustineKoa commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r447181488



##########
File path: beam-ci/pom.xml
##########
@@ -0,0 +1,45 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>org.jenkins-ci.plugins</groupId>
+        <artifactId>plugin</artifactId>
+        <version>3.50</version>
+        <relativePath />
+    </parent>
+    <groupId>io.jenkins.plugins</groupId>
+    <artifactId>beam-ci</artifactId>
+    <version>1.0-SNAPSHOT</version>
+    <packaging>hpi</packaging>
+    <properties>
+        <jenkins.version>2.176.4</jenkins.version>
+        <java.level>8</java.level>
+    </properties>
+    <name>Beam CI</name>
+    <licenses>
+        <license>
+            <name>MIT License</name>
+            <url>https://opensource.org/licenses/MIT</url>

Review comment:
       Replacing the MIT license with this:
   ```
           <license>
               <name>Apache Beam License</name>
               <url>http://www.apache.org/licenses/LICENSE-2.0</url>
           </license>
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] stale[bot] commented on pull request #12042: [BEAM-9962] Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-723500384


   This pull request has been marked as stale due to 60 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@beam.apache.org list. Thank you for your contributions.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] angoenka commented on a change in pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
angoenka commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r446830218



##########
File path: beam-ci/src/main/resources/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder/config.jelly
##########
@@ -0,0 +1,36 @@
+<?jelly escape-by-default='true'?>

Review comment:
       Can we move these files to 'plugins' directory if possible.

##########
File path: beam-ci/src/main/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder.java
##########
@@ -0,0 +1,173 @@
+package io.jenkins.plugins;
+
+import hudson.Launcher;
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.util.FormValidation;
+import hudson.model.AbstractProject;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.tasks.Builder;
+import hudson.tasks.BuildStepDescriptor;
+import jenkins.util.SystemProperties;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
+
+import javax.servlet.ServletException;
+import java.io.BufferedReader;
+import java.io.File;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.util.*;
+
+import jenkins.tasks.SimpleBuildStep;
+
+public class ExecuteBeamPipelineOnDataflowBuilder extends Builder implements SimpleBuildStep {
+
+    private final String pathToCreds;
+    private final String pathToMainClass;
+    private final String pipelineOptions;
+    private final String buildReleaseOptions;
+    private boolean useJava; // if false, use Python
+    private boolean useGradle; // if false, use Maven
+
+    @DataBoundConstructor
+    public ExecuteBeamPipelineOnDataflowBuilder(String pathToCreds, String pathToMainClass, String pipelineOptions, String buildReleaseOptions, boolean useJava, boolean useGradle) {
+        this.pathToCreds = pathToCreds;
+        this.pathToMainClass = pathToMainClass;
+        this.pipelineOptions = pipelineOptions;
+        this.buildReleaseOptions = buildReleaseOptions;
+        this.useJava = useJava;
+        this.useGradle = useGradle;
+    }
+
+    public String getPathToCreds() { return pathToCreds; }
+
+    public String getPathToMainClass() {
+        return pathToMainClass;
+    }
+
+    public String getPipelineOptions() {
+        return pipelineOptions;
+    }
+
+    public String getBuildReleaseOptions() {
+        return buildReleaseOptions;
+    }
+
+    public boolean getUseJava() {
+        return useJava;
+    }
+
+    public boolean getUseGradle() {
+        return useGradle;
+    }
+
+    /**
+     * Builds and sets the command on the ProcessBuilder depending on configurations set by the user
+     * */
+    private void buildCommand(Run<?, ?> run, String workspace, ProcessBuilder processBuilder) {
+        ArrayList<String> command;
+        if (this.useJava) {
+            String pipelineOptions = this.pipelineOptions.replaceAll("[\\t\\n]+"," ");
+            if (this.useGradle) { // gradle
+                command = new ArrayList<>(Arrays.asList("gradle", "clean", "execute", "-DmainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            } else { // maven
+                command = new ArrayList<>(Arrays.asList("mvn", "compile", "exec:java", "-Dexec.mainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            }
+
+            // add pipeline and build release options if included
+            if (!this.buildReleaseOptions.equals("")) {
+                String[] buildReleaseOptions = this.buildReleaseOptions.split("\\s+"); // split build release options by whitespace
+                command.addAll(Arrays.asList(buildReleaseOptions)); // add build release options as separate list elements
+            }
+            //        System.out.println(Arrays.toString(command.toArray()));
+            processBuilder.command(command);
+        } else { // python
+            // Get Path to the Bash Script
+            String dir = System.getProperty("user.dir"); // Get the directory the plugin is located
+            String pathToScript = dir + "/src/main/java/io/jenkins/plugins/executePythonBeamPipeline.sh";
+
+            // Get Path to the current build directory to create Virtual Environment in
+            String jobBuildPathDirectory = getJobBuildDirectory(run);
+
+            // Execute Bash Script
+            processBuilder.command(pathToScript, workspace, jobBuildPathDirectory, this.pathToMainClass, this.pipelineOptions, this.buildReleaseOptions);
+        }
+    }
+
+    /**
+     * @return absolute path to current build folder
+     * */
+    private String getJobBuildDirectory(Run<?, ?> run) {
+        String jenkinsHome = System.getProperty("JENKINS_HOME");
+        String jobName = run.getFullDisplayName().split(" ")[0];
+        int buildNumber = run.getNumber();
+        return jenkinsHome + "/jobs/" + jobName + "/builds/" + buildNumber;
+    }
+
+    @Override
+    public void perform(Run<?, ?> run, FilePath workspace, Launcher launcher, TaskListener listener) throws InterruptedException, IOException {
+        ProcessBuilder processBuilder = new ProcessBuilder();
+
+        // see that all configurations are received correctly
+        listener.getLogger().println("path to google app creds : " + this.pathToCreds);

Review comment:
       google app creds -> google cloud credentials

##########
File path: beam-ci/src/main/resources/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder/help-buildReleaseOptions.html
##########
@@ -0,0 +1,4 @@
+<div>
+    Include optional build release options. <br/>
+    <b>Maven and Gradle example</b>: -Pdataflow-runner
+</div>

Review comment:
       Let's add new lines to all the files.

##########
File path: beam-ci/pom.xml
##########
@@ -0,0 +1,45 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>org.jenkins-ci.plugins</groupId>
+        <artifactId>plugin</artifactId>
+        <version>3.50</version>
+        <relativePath />
+    </parent>
+    <groupId>io.jenkins.plugins</groupId>
+    <artifactId>beam-ci</artifactId>
+    <version>1.0-SNAPSHOT</version>
+    <packaging>hpi</packaging>
+    <properties>
+        <jenkins.version>2.176.4</jenkins.version>
+        <java.level>8</java.level>
+    </properties>
+    <name>Beam CI</name>
+    <licenses>
+        <license>
+            <name>MIT License</name>
+            <url>https://opensource.org/licenses/MIT</url>

Review comment:
       We will need apache as Beam is under the same license https://www.apache.org/licenses/LICENSE-2.0 

##########
File path: beam-ci/src/main/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder.java
##########
@@ -0,0 +1,173 @@
+package io.jenkins.plugins;

Review comment:
       Add the license header to all the new files https://www.apache.org/legal/src-headers.html#headers
   Example https://github.com/apache/beam/blob/e68397c8cc834bdd62ef2de5f5c1476d9c456b9d/sdks/java/core/src/main/java/org/apache/beam/sdk/function/ThrowingConsumer.java#L1




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] JustineKoa commented on a change in pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
JustineKoa commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r447181488



##########
File path: beam-ci/pom.xml
##########
@@ -0,0 +1,45 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>org.jenkins-ci.plugins</groupId>
+        <artifactId>plugin</artifactId>
+        <version>3.50</version>
+        <relativePath />
+    </parent>
+    <groupId>io.jenkins.plugins</groupId>
+    <artifactId>beam-ci</artifactId>
+    <version>1.0-SNAPSHOT</version>
+    <packaging>hpi</packaging>
+    <properties>
+        <jenkins.version>2.176.4</jenkins.version>
+        <java.level>8</java.level>
+    </properties>
+    <name>Beam CI</name>
+    <licenses>
+        <license>
+            <name>MIT License</name>
+            <url>https://opensource.org/licenses/MIT</url>

Review comment:
       Would this entail adding this after the MIT License? 
   ```
           <license>
               <name>Apache Beam License</name>
               <url>http://www.apache.org/licenses/LICENSE-2.0</url>
           </license>
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] aaltay commented on pull request #12042: [BEAM-9962] Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-666767694


   What are the next steps for this PR?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] JustineKoa commented on a change in pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
JustineKoa commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r447190465



##########
File path: beam-ci/src/main/resources/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder/help-buildReleaseOptions.html
##########
@@ -0,0 +1,4 @@
+<div>
+    Include optional build release options. <br/>
+    <b>Maven and Gradle example</b>: -Pdataflow-runner
+</div>

Review comment:
       Hmmm I'm not sure what you mean by this. New lines instead of using <br/>?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] stale[bot] commented on pull request #12042: [BEAM-9962] Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
stale[bot] commented on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-727715867


   This pull request has been closed due to lack of activity. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] stale[bot] closed pull request #12042: [BEAM-9962] Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
stale[bot] closed pull request #12042:
URL: https://github.com/apache/beam/pull/12042


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] JustineKoa commented on a change in pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
JustineKoa commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r448523037



##########
File path: beam-ci/src/main/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.jenkins.plugins;
+
+import hudson.Launcher;
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.util.FormValidation;
+import hudson.model.AbstractProject;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.tasks.Builder;
+import hudson.tasks.BuildStepDescriptor;
+import jenkins.util.SystemProperties;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
+
+import javax.servlet.ServletException;
+import java.io.*;
+import java.util.*;
+
+import jenkins.tasks.SimpleBuildStep;
+
+public class ExecuteBeamPipelineOnDataflowBuilder extends Builder implements SimpleBuildStep {
+
+    private ProcessBuilder processBuilder;
+    private final String pathToCreds;
+    private final String pathToMainClass;
+    private final String pipelineOptions;
+    private final String buildReleaseOptions;
+    private boolean useJava; // if false, use Python
+    private boolean useGradle; // if false, use Maven
+    private ArrayList<String> command;
+    private boolean test; // if we are testing, the mock ProcessBuilder never starts
+
+    @DataBoundConstructor
+    public ExecuteBeamPipelineOnDataflowBuilder(String pathToCreds, String pathToMainClass, String pipelineOptions, String buildReleaseOptions, boolean useJava, boolean useGradle) {
+        this.pathToCreds = pathToCreds;
+        this.pathToMainClass = pathToMainClass;
+        this.pipelineOptions = pipelineOptions;
+        this.buildReleaseOptions = buildReleaseOptions;
+        this.useJava = useJava;
+        this.useGradle = useGradle;
+        this.test = false;
+    }
+
+    public void setProcessBuilder(ProcessBuilder processBuilder) {

Review comment:
       Oh wow I didn't know these are made by default! Thanks




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] JustineKoa commented on a change in pull request #12042: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
JustineKoa commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r448536674



##########
File path: beam-ci/src/main/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.jenkins.plugins;
+
+import hudson.Launcher;
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.util.FormValidation;
+import hudson.model.AbstractProject;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.tasks.Builder;
+import hudson.tasks.BuildStepDescriptor;
+import jenkins.util.SystemProperties;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
+
+import javax.servlet.ServletException;
+import java.io.*;
+import java.util.*;
+
+import jenkins.tasks.SimpleBuildStep;
+
+public class ExecuteBeamPipelineOnDataflowBuilder extends Builder implements SimpleBuildStep {
+
+    private ProcessBuilder processBuilder;
+    private final String pathToCreds;
+    private final String pathToMainClass;
+    private final String pipelineOptions;
+    private final String buildReleaseOptions;
+    private boolean useJava; // if false, use Python
+    private boolean useGradle; // if false, use Maven
+    private ArrayList<String> command;
+    private boolean test; // if we are testing, the mock ProcessBuilder never starts
+
+    @DataBoundConstructor
+    public ExecuteBeamPipelineOnDataflowBuilder(String pathToCreds, String pathToMainClass, String pipelineOptions, String buildReleaseOptions, boolean useJava, boolean useGradle) {
+        this.pathToCreds = pathToCreds;
+        this.pathToMainClass = pathToMainClass;
+        this.pipelineOptions = pipelineOptions;
+        this.buildReleaseOptions = buildReleaseOptions;
+        this.useJava = useJava;
+        this.useGradle = useGradle;
+        this.test = false;
+    }
+
+    public void setProcessBuilder(ProcessBuilder processBuilder) {
+        this.processBuilder = processBuilder;
+        this.test = true;
+    }
+
+    public String getPathToCreds() { return pathToCreds; }
+
+    public String getPathToMainClass() {
+        return pathToMainClass;
+    }
+
+    public String getPipelineOptions() {
+        return pipelineOptions;
+    }
+
+    public String getBuildReleaseOptions() {
+        return buildReleaseOptions;
+    }
+
+    public boolean getUseJava() {
+        return useJava;
+    }
+
+    public boolean getUseGradle() {
+        return useGradle;
+    }
+
+    public ArrayList<String> getCommand() { return command; }
+
+    /**
+     * Builds and sets the command on the ProcessBuilder depending on configurations set by the user
+     * */
+    private void buildCommand(Run<?, ?> run, String workspace, PrintStream logger) {
+//        ArrayList<String> command;
+        if (this.useJava) {
+            String pipelineOptions = this.pipelineOptions.replaceAll("[\\t\\n]+"," ");
+            if (this.useGradle) { // gradle
+                command = new ArrayList<>(Arrays.asList("gradle", "clean", "execute", "-DmainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            } else { // maven
+                command = new ArrayList<>(Arrays.asList("mvn", "compile", "exec:java", "-Dexec.mainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            }
+
+            // add pipeline and build release options if included
+            if (!this.buildReleaseOptions.equals("")) {
+                String[] buildReleaseOptions = this.buildReleaseOptions.split("\\s+"); // split build release options by whitespace
+                command.addAll(Arrays.asList(buildReleaseOptions)); // add build release options as separate list elements
+            }
+        } else { // python
+            // Get Path to the Bash Script
+            String dir = System.getProperty("user.dir"); // Get the directory the plugin is located
+            String pathToScript = dir + "/src/main/java/io/jenkins/plugins/executePythonBeamPipeline.sh";
+
+            // Get Path to the current build directory to create Virtual Environment in
+            String jobBuildPathDirectory = getJobBuildDirectory(run);
+
+            // Execute Bash Script
+            command = new ArrayList<>(Arrays.asList(pathToScript, workspace, jobBuildPathDirectory, this.pathToMainClass, this.pipelineOptions, this.buildReleaseOptions));
+        }
+        this.processBuilder.command(command);
+    }
+
+    /**
+     * @return absolute path to current build folder
+     * */
+    private String getJobBuildDirectory(Run<?, ?> run) {
+        String jenkinsHome = System.getProperty("JENKINS_HOME");
+        String jobName = run.getFullDisplayName().split(" ")[0];
+        int buildNumber = run.getNumber();
+        return jenkinsHome + "/jobs/" + jobName + "/builds/" + buildNumber;
+    }
+
+    @Override
+    public void perform(Run<?, ?> run, FilePath workspace, Launcher launcher, TaskListener listener) throws InterruptedException, IOException {
+//        ProcessBuilder processBuilder = new ProcessBuilder();
+        if (processBuilder == null) {
+            this.processBuilder = new ProcessBuilder();
+        }
+
+        // see that all configurations are received correctly
+        listener.getLogger().println("path to google cloud credentials : " + this.pathToCreds);
+        listener.getLogger().println("path to main class : " + this.pathToMainClass);
+        listener.getLogger().println("pipeline options : " + this.pipelineOptions);
+        listener.getLogger().println("build release options : " + this.buildReleaseOptions);
+        listener.getLogger().println("use java: " + this.useJava);
+        listener.getLogger().println("use gradle: " + this.useGradle);
+
+        Map<String, String> env = this.processBuilder.environment();
+        env.put("GOOGLE_APPLICATION_CREDENTIALS", this.pathToCreds);
+
+        // set correct directory to be running command
+        this.processBuilder.directory(new File(workspace.toURI()));
+
+        // build and set command to processBuilder based on configurations
+        buildCommand(run, workspace.toURI().getPath(), listener.getLogger());
+
+        if (this.test) { // if we are testing commands only, return before starting process

Review comment:
       The reason I'm asking/a bit confused is because I create BufferedReader depending on the process InputStream and ErrorStream, so I'm not sure how I can set it to be a mock in the tests.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] JustineKoa commented on a change in pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
JustineKoa commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r448524865



##########
File path: beam-ci/src/main/resources/index.jelly
##########
@@ -0,0 +1,4 @@
+<?jelly escape-by-default='true'?>
+<div>
+    TODO

Review comment:
       This was generated automatically from the starter template for a Jenkins plugin! I never found that I needed to edit this file but at the same time, I'm not sure if deleting it was the right thing to do.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12042: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-652683753


   High-level note: I might have missed a discussion or a design but from skimming through the PR description it is not clear what the purpose of Jenkins plugin is. It would be helpful to add some information. Also, Tobiasz (TobKed@)  has recently been working a lot on Jenkins infra and may have valuable feedback here. Consider including Tobiasz once this is ready for review.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] JustineKoa commented on a change in pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
JustineKoa commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r448524480



##########
File path: beam-ci/src/main/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.jenkins.plugins;
+
+import hudson.Launcher;
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.util.FormValidation;
+import hudson.model.AbstractProject;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.tasks.Builder;
+import hudson.tasks.BuildStepDescriptor;
+import jenkins.util.SystemProperties;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
+
+import javax.servlet.ServletException;
+import java.io.*;
+import java.util.*;
+
+import jenkins.tasks.SimpleBuildStep;
+
+public class ExecuteBeamPipelineOnDataflowBuilder extends Builder implements SimpleBuildStep {
+
+    private ProcessBuilder processBuilder;
+    private final String pathToCreds;
+    private final String pathToMainClass;
+    private final String pipelineOptions;
+    private final String buildReleaseOptions;
+    private boolean useJava; // if false, use Python
+    private boolean useGradle; // if false, use Maven
+    private ArrayList<String> command;
+    private boolean test; // if we are testing, the mock ProcessBuilder never starts
+
+    @DataBoundConstructor
+    public ExecuteBeamPipelineOnDataflowBuilder(String pathToCreds, String pathToMainClass, String pipelineOptions, String buildReleaseOptions, boolean useJava, boolean useGradle) {
+        this.pathToCreds = pathToCreds;
+        this.pathToMainClass = pathToMainClass;
+        this.pipelineOptions = pipelineOptions;
+        this.buildReleaseOptions = buildReleaseOptions;
+        this.useJava = useJava;
+        this.useGradle = useGradle;
+        this.test = false;
+    }
+
+    public void setProcessBuilder(ProcessBuilder processBuilder) {
+        this.processBuilder = processBuilder;
+        this.test = true;
+    }
+
+    public String getPathToCreds() { return pathToCreds; }
+
+    public String getPathToMainClass() {
+        return pathToMainClass;
+    }
+
+    public String getPipelineOptions() {
+        return pipelineOptions;
+    }
+
+    public String getBuildReleaseOptions() {
+        return buildReleaseOptions;
+    }
+
+    public boolean getUseJava() {
+        return useJava;
+    }
+
+    public boolean getUseGradle() {
+        return useGradle;
+    }
+
+    public ArrayList<String> getCommand() { return command; }
+
+    /**
+     * Builds and sets the command on the ProcessBuilder depending on configurations set by the user
+     * */
+    private void buildCommand(Run<?, ?> run, String workspace, PrintStream logger) {
+//        ArrayList<String> command;
+        if (this.useJava) {
+            String pipelineOptions = this.pipelineOptions.replaceAll("[\\t\\n]+"," ");
+            if (this.useGradle) { // gradle
+                command = new ArrayList<>(Arrays.asList("gradle", "clean", "execute", "-DmainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            } else { // maven
+                command = new ArrayList<>(Arrays.asList("mvn", "compile", "exec:java", "-Dexec.mainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            }
+
+            // add pipeline and build release options if included
+            if (!this.buildReleaseOptions.equals("")) {
+                String[] buildReleaseOptions = this.buildReleaseOptions.split("\\s+"); // split build release options by whitespace
+                command.addAll(Arrays.asList(buildReleaseOptions)); // add build release options as separate list elements
+            }
+        } else { // python
+            // Get Path to the Bash Script
+            String dir = System.getProperty("user.dir"); // Get the directory the plugin is located
+            String pathToScript = dir + "/src/main/java/io/jenkins/plugins/executePythonBeamPipeline.sh";
+
+            // Get Path to the current build directory to create Virtual Environment in
+            String jobBuildPathDirectory = getJobBuildDirectory(run);
+
+            // Execute Bash Script
+            command = new ArrayList<>(Arrays.asList(pathToScript, workspace, jobBuildPathDirectory, this.pathToMainClass, this.pipelineOptions, this.buildReleaseOptions));
+        }
+        this.processBuilder.command(command);
+    }
+
+    /**
+     * @return absolute path to current build folder
+     * */
+    private String getJobBuildDirectory(Run<?, ?> run) {
+        String jenkinsHome = System.getProperty("JENKINS_HOME");
+        String jobName = run.getFullDisplayName().split(" ")[0];
+        int buildNumber = run.getNumber();
+        return jenkinsHome + "/jobs/" + jobName + "/builds/" + buildNumber;
+    }
+
+    @Override
+    public void perform(Run<?, ?> run, FilePath workspace, Launcher launcher, TaskListener listener) throws InterruptedException, IOException {
+//        ProcessBuilder processBuilder = new ProcessBuilder();
+        if (processBuilder == null) {
+            this.processBuilder = new ProcessBuilder();
+        }
+
+        // see that all configurations are received correctly
+        listener.getLogger().println("path to google cloud credentials : " + this.pathToCreds);
+        listener.getLogger().println("path to main class : " + this.pathToMainClass);
+        listener.getLogger().println("pipeline options : " + this.pipelineOptions);
+        listener.getLogger().println("build release options : " + this.buildReleaseOptions);
+        listener.getLogger().println("use java: " + this.useJava);
+        listener.getLogger().println("use gradle: " + this.useGradle);
+
+        Map<String, String> env = this.processBuilder.environment();
+        env.put("GOOGLE_APPLICATION_CREDENTIALS", this.pathToCreds);
+
+        // set correct directory to be running command
+        this.processBuilder.directory(new File(workspace.toURI()));
+
+        // build and set command to processBuilder based on configurations
+        buildCommand(run, workspace.toURI().getPath(), listener.getLogger());
+
+        if (this.test) { // if we are testing commands only, return before starting process

Review comment:
       I agree! Does this mean I should mock the BufferedReaders as well?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] aaltay commented on pull request #12042: [BEAM-9962] Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
aaltay commented on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-673680257


   @angoenka - should this be merged?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] angoenka commented on pull request #12042: [BEAM-9962] Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
angoenka commented on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-675761863


   Based on @TobKed suggestion, I was planning to work on it but it will be difficult to get to it in this quarter.
   @TobKed shall we merge it as it is to make these changes available?
   Jenkins plugin will still not be a supported feature and will be work in progress.
   We will also plan to migrate beam tests to jenkins plugin later.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] TobKed commented on pull request #12042: [BEAM-9962] Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
TobKed commented on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-683420422


   @angoenka sorry for the late response.
   I think it would be better to do not merge it since it is not finished yet. As @tvalentyn mentioned, it would be better to do not introduce any technical debt. You may put [WIP] prefix in the PR title and work on it when you have more time.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] angoenka commented on a change in pull request #12042: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
angoenka commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r448534456



##########
File path: beam-ci/src/main/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.jenkins.plugins;
+
+import hudson.Launcher;
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.util.FormValidation;
+import hudson.model.AbstractProject;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.tasks.Builder;
+import hudson.tasks.BuildStepDescriptor;
+import jenkins.util.SystemProperties;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
+
+import javax.servlet.ServletException;
+import java.io.*;
+import java.util.*;
+
+import jenkins.tasks.SimpleBuildStep;
+
+public class ExecuteBeamPipelineOnDataflowBuilder extends Builder implements SimpleBuildStep {
+
+    private ProcessBuilder processBuilder;
+    private final String pathToCreds;
+    private final String pathToMainClass;
+    private final String pipelineOptions;
+    private final String buildReleaseOptions;
+    private boolean useJava; // if false, use Python
+    private boolean useGradle; // if false, use Maven
+    private ArrayList<String> command;
+    private boolean test; // if we are testing, the mock ProcessBuilder never starts
+
+    @DataBoundConstructor
+    public ExecuteBeamPipelineOnDataflowBuilder(String pathToCreds, String pathToMainClass, String pipelineOptions, String buildReleaseOptions, boolean useJava, boolean useGradle) {
+        this.pathToCreds = pathToCreds;
+        this.pathToMainClass = pathToMainClass;
+        this.pipelineOptions = pipelineOptions;
+        this.buildReleaseOptions = buildReleaseOptions;
+        this.useJava = useJava;
+        this.useGradle = useGradle;
+        this.test = false;
+    }
+
+    public void setProcessBuilder(ProcessBuilder processBuilder) {
+        this.processBuilder = processBuilder;
+        this.test = true;
+    }
+
+    public String getPathToCreds() { return pathToCreds; }
+
+    public String getPathToMainClass() {
+        return pathToMainClass;
+    }
+
+    public String getPipelineOptions() {
+        return pipelineOptions;
+    }
+
+    public String getBuildReleaseOptions() {
+        return buildReleaseOptions;
+    }
+
+    public boolean getUseJava() {
+        return useJava;
+    }
+
+    public boolean getUseGradle() {
+        return useGradle;
+    }
+
+    public ArrayList<String> getCommand() { return command; }
+
+    /**
+     * Builds and sets the command on the ProcessBuilder depending on configurations set by the user
+     * */
+    private void buildCommand(Run<?, ?> run, String workspace, PrintStream logger) {
+//        ArrayList<String> command;
+        if (this.useJava) {
+            String pipelineOptions = this.pipelineOptions.replaceAll("[\\t\\n]+"," ");
+            if (this.useGradle) { // gradle
+                command = new ArrayList<>(Arrays.asList("gradle", "clean", "execute", "-DmainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            } else { // maven
+                command = new ArrayList<>(Arrays.asList("mvn", "compile", "exec:java", "-Dexec.mainClass=" + this.pathToMainClass, "-Dexec.args=" + pipelineOptions));
+            }
+
+            // add pipeline and build release options if included
+            if (!this.buildReleaseOptions.equals("")) {
+                String[] buildReleaseOptions = this.buildReleaseOptions.split("\\s+"); // split build release options by whitespace
+                command.addAll(Arrays.asList(buildReleaseOptions)); // add build release options as separate list elements
+            }
+        } else { // python
+            // Get Path to the Bash Script
+            String dir = System.getProperty("user.dir"); // Get the directory the plugin is located
+            String pathToScript = dir + "/src/main/java/io/jenkins/plugins/executePythonBeamPipeline.sh";
+
+            // Get Path to the current build directory to create Virtual Environment in
+            String jobBuildPathDirectory = getJobBuildDirectory(run);
+
+            // Execute Bash Script
+            command = new ArrayList<>(Arrays.asList(pathToScript, workspace, jobBuildPathDirectory, this.pathToMainClass, this.pipelineOptions, this.buildReleaseOptions));
+        }
+        this.processBuilder.command(command);
+    }
+
+    /**
+     * @return absolute path to current build folder
+     * */
+    private String getJobBuildDirectory(Run<?, ?> run) {
+        String jenkinsHome = System.getProperty("JENKINS_HOME");
+        String jobName = run.getFullDisplayName().split(" ")[0];
+        int buildNumber = run.getNumber();
+        return jenkinsHome + "/jobs/" + jobName + "/builds/" + buildNumber;
+    }
+
+    @Override
+    public void perform(Run<?, ?> run, FilePath workspace, Launcher launcher, TaskListener listener) throws InterruptedException, IOException {
+//        ProcessBuilder processBuilder = new ProcessBuilder();
+        if (processBuilder == null) {
+            this.processBuilder = new ProcessBuilder();
+        }
+
+        // see that all configurations are received correctly
+        listener.getLogger().println("path to google cloud credentials : " + this.pathToCreds);
+        listener.getLogger().println("path to main class : " + this.pathToMainClass);
+        listener.getLogger().println("pipeline options : " + this.pipelineOptions);
+        listener.getLogger().println("build release options : " + this.buildReleaseOptions);
+        listener.getLogger().println("use java: " + this.useJava);
+        listener.getLogger().println("use gradle: " + this.useGradle);
+
+        Map<String, String> env = this.processBuilder.environment();
+        env.put("GOOGLE_APPLICATION_CREDENTIALS", this.pathToCreds);
+
+        // set correct directory to be running command
+        this.processBuilder.directory(new File(workspace.toURI()));
+
+        // build and set command to processBuilder based on configurations
+        buildCommand(run, workspace.toURI().getPath(), listener.getLogger());
+
+        if (this.test) { // if we are testing commands only, return before starting process

Review comment:
       Not really, You can return a valid buffered reader from process.getInputStream. But this means that you will have to mock porcess as well.

##########
File path: beam-ci/src/main/resources/index.jelly
##########
@@ -0,0 +1,4 @@
+<?jelly escape-by-default='true'?>
+<div>
+    TODO

Review comment:
       https://www.jenkins.io/doc/developer/tutorial/extend/ might be helpful.
   
   Let's TODO with the name of the plugin.

##########
File path: beam-ci/src/main/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.jenkins.plugins;
+
+import hudson.Launcher;
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.util.FormValidation;
+import hudson.model.AbstractProject;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.tasks.Builder;
+import hudson.tasks.BuildStepDescriptor;
+import jenkins.util.SystemProperties;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
+
+import javax.servlet.ServletException;
+import java.io.*;
+import java.util.*;
+
+import jenkins.tasks.SimpleBuildStep;
+
+public class ExecuteBeamPipelineOnDataflowBuilder extends Builder implements SimpleBuildStep {
+
+    private ProcessBuilder processBuilder;
+    private final String pathToCreds;
+    private final String pathToMainClass;
+    private final String pipelineOptions;
+    private final String buildReleaseOptions;
+    private boolean useJava; // if false, use Python
+    private boolean useGradle; // if false, use Maven
+    private ArrayList<String> command;
+    private boolean test; // if we are testing, the mock ProcessBuilder never starts
+
+    @DataBoundConstructor
+    public ExecuteBeamPipelineOnDataflowBuilder(String pathToCreds, String pathToMainClass, String pipelineOptions, String buildReleaseOptions, boolean useJava, boolean useGradle) {
+        this.pathToCreds = pathToCreds;
+        this.pathToMainClass = pathToMainClass;
+        this.pipelineOptions = pipelineOptions;
+        this.buildReleaseOptions = buildReleaseOptions;
+        this.useJava = useJava;
+        this.useGradle = useGradle;
+        this.test = false;
+    }
+
+    public void setProcessBuilder(ProcessBuilder processBuilder) {

Review comment:
       Yes, we don't need to make it public as it's only accessed by test which are in same packages.
   Also, see below comment to remove this method whole together.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] JustineKoa commented on a change in pull request #12042: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
JustineKoa commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r448599468



##########
File path: beam-ci/src/main/resources/index.jelly
##########
@@ -0,0 +1,4 @@
+<?jelly escape-by-default='true'?>
+<div>
+    TODO

Review comment:
       I found out that "This view is used to render the installed plugins page." So I'm keeping it and just adding in the div what the plugin is.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12042: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-652683956


   Also, please add the Jira to the PR title.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] angoenka commented on a change in pull request #12042: [BEAM-9962] Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
angoenka commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r448673087



##########
File path: beam-ci/src/main/resources/index.jelly
##########
@@ -0,0 +1,7 @@
+<?jelly escape-by-default='true'?>

Review comment:
       Let's add the license here.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12042: [BEAM-9962] Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-684076172


   > You may put [WIP] prefix in the PR title.
   I marked this as draft PR.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] angoenka commented on pull request #12042: [BEAM-9962] Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
angoenka commented on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-654516184


   Thanks @TobKed  for the input. 
   1. Yes, I agree we should have a document on beam website for the instructions.
   2. I plan to move the plugin to Gradle later if possible and then we can add the test trigger.
   3. I plan to release it with each Beam release but we have not yet though through it.
   4. `beam-jenkins-plugin` sounds better to me.
   
   Shall I note these feedback in a jira, get the PR in and then take the action items from the jira once I get some time?
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] JustineKoa commented on a change in pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
JustineKoa commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r447186469



##########
File path: beam-ci/src/main/resources/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder/config.jelly
##########
@@ -0,0 +1,36 @@
+<?jelly escape-by-default='true'?>

Review comment:
       I think it is jelly convention to put it in the directory named after the Builder it is tied to. I believe that's how the project is able to identify where the jelly files are for the Builder class.

##########
File path: beam-ci/src/main/resources/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder/config.jelly
##########
@@ -0,0 +1,36 @@
+<?jelly escape-by-default='true'?>

Review comment:
       I think it is Jenkins convention to put it in the directory named after the Builder it is tied to. I believe that's how the project is able to identify where the jelly files are for the Builder class.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] JustineKoa commented on a change in pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
JustineKoa commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r448522058



##########
File path: beam-ci/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker
##########
@@ -0,0 +1 @@
+mock-maker-inline

Review comment:
       Otherwise I get errors for making a mock of ProcessBuilder (because it is a final class)




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] angoenka commented on pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
angoenka commented on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-651573889


   R: @tvalentyn Would you also take a look at the PR from Jenkins point of view.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] JustineKoa commented on a change in pull request #12042: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
JustineKoa commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r448522058



##########
File path: beam-ci/src/test/resources/mockito-extensions/org.mockito.plugins.MockMaker
##########
@@ -0,0 +1 @@
+mock-maker-inline

Review comment:
       Otherwise I get errors for making a mock of ProcessBuilder (because it is a final class)
   
   Update: was able to remove this after adding wrapper!




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] angoenka commented on pull request #12042: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
angoenka commented on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-652697557


   cc: @TobKed PTAL 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] TobKed commented on pull request #12042: [BEAM-9962] Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
TobKed commented on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-657406084


   > Shall I note these feedback in a jira, get the PR in and then take the action items from the jira once I get some time?
   
   I am not sure is it necessary. I think you can work in this PR on most of them, then eventually create another jira and PR with some comments/links related to this PR.
   
   WDYT @angoenka @tvalentyn ?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] tvalentyn commented on pull request #12042: [BEAM-9962] Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
tvalentyn commented on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-659839466


   @angoenka I did not have time to take a deeper look at the PR yet. Re whether to merge now or later:
   If the PR adds value and is in reasonable shape, we can merge and iterate.  If this PR is not yet ready to merge (for example adds code but we also need tests, or introduces some technical debt that needs to be fixed immediately), you can add commits to this PR that you plan, and we can merge after that. By default Beam committers are able to add new commits to PRs unless explicitly disabled by branch owner. 


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] JustineKoa commented on a change in pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
JustineKoa commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r448523037



##########
File path: beam-ci/src/main/java/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder.java
##########
@@ -0,0 +1,206 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.jenkins.plugins;
+
+import hudson.Launcher;
+import hudson.Extension;
+import hudson.FilePath;
+import hudson.util.FormValidation;
+import hudson.model.AbstractProject;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.tasks.Builder;
+import hudson.tasks.BuildStepDescriptor;
+import jenkins.util.SystemProperties;
+import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.QueryParameter;
+
+import javax.servlet.ServletException;
+import java.io.*;
+import java.util.*;
+
+import jenkins.tasks.SimpleBuildStep;
+
+public class ExecuteBeamPipelineOnDataflowBuilder extends Builder implements SimpleBuildStep {
+
+    private ProcessBuilder processBuilder;
+    private final String pathToCreds;
+    private final String pathToMainClass;
+    private final String pipelineOptions;
+    private final String buildReleaseOptions;
+    private boolean useJava; // if false, use Python
+    private boolean useGradle; // if false, use Maven
+    private ArrayList<String> command;
+    private boolean test; // if we are testing, the mock ProcessBuilder never starts
+
+    @DataBoundConstructor
+    public ExecuteBeamPipelineOnDataflowBuilder(String pathToCreds, String pathToMainClass, String pipelineOptions, String buildReleaseOptions, boolean useJava, boolean useGradle) {
+        this.pathToCreds = pathToCreds;
+        this.pathToMainClass = pathToMainClass;
+        this.pipelineOptions = pipelineOptions;
+        this.buildReleaseOptions = buildReleaseOptions;
+        this.useJava = useJava;
+        this.useGradle = useGradle;
+        this.test = false;
+    }
+
+    public void setProcessBuilder(ProcessBuilder processBuilder) {

Review comment:
       Edit: I'm looked into default access modifiers and I'm not 100% sure I understand what you mean. Does this mean I shouldn't be making this a public method?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] tvalentyn edited a comment on pull request #12042: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
tvalentyn edited a comment on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-652683956


   Also, please add a Jira issue to the PR title.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] JustineKoa commented on a change in pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
JustineKoa commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r447190465



##########
File path: beam-ci/src/main/resources/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder/help-buildReleaseOptions.html
##########
@@ -0,0 +1,4 @@
+<div>
+    Include optional build release options. <br/>
+    <b>Maven and Gradle example</b>: -Pdataflow-runner
+</div>

Review comment:
       Hmmm I'm not sure what you mean by this. New lines instead of using ```<br/>```?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] angoenka commented on a change in pull request #12042: [BEAM-9962] Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
angoenka commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r448671816



##########
File path: beam-ci/src/main/java/io/jenkins/plugins/PipelineLauncher.java
##########
@@ -0,0 +1,74 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.jenkins.plugins;
+
+import java.io.*;
+import java.util.List;
+import java.util.Map;
+
+public class PipelineLauncher {
+
+    private ProcessBuilder processBuilder;
+
+    public PipelineLauncher(String pathToCredentials, File workspace) {
+        this.processBuilder = new ProcessBuilder();
+
+        // add environment variable
+        Map<String, String> env = this.processBuilder.environment();
+        env.put("GOOGLE_APPLICATION_CREDENTIALS", pathToCredentials);
+
+        // set correct directory to be running command
+        this.processBuilder.directory(workspace);
+    }
+
+    public void command(List<String> command) {
+        this.processBuilder.command(command);
+    }
+
+    public LaunchProcess start() throws IOException {
+        Process process = this.processBuilder.start();
+        return new LaunchProcess(process);
+    }
+
+    public static class LaunchProcess {

Review comment:
       LaunchProcess  -> LaunchedProcess




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] JustineKoa commented on a change in pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
JustineKoa commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r447190465



##########
File path: beam-ci/src/main/resources/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder/help-buildReleaseOptions.html
##########
@@ -0,0 +1,4 @@
+<div>
+    Include optional build release options. <br/>
+    <b>Maven and Gradle example</b>: -Pdataflow-runner
+</div>

Review comment:
       Hmmm I'm not sure what you mean by this. New lines instead of using ```<br/>```?
   Edit: Got it! Adding newlines to the end of the files




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] angoenka commented on pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
angoenka commented on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-652295930


   If it's ok, let's remove the draft label and WIP from the PR


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] TobKed commented on pull request #12042: [BEAM-9962] Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
TobKed commented on pull request #12042:
URL: https://github.com/apache/beam/pull/12042#issuecomment-653742927


   Hi, sorry for the late response.
   
   I am not familiar with the creation of the custom plugins for Jenkins however I took briefly on it and I have some more general questions:
    1. Is it possible to attach some documentation to the Beam website about this plugin? Maybe some kind of readme.md the purpose/description/instructions may be helpful.
    2. Is it possible to include tests for this plugin in CI? e.g. to run tests for the plugin if anything in `beam-ci/` path changed (`_Commit` jobs).
    3. How the versions/releases of the plugin will be handled?
    4. I'm thinking about the name. My first impression was that I was confused with `beam-ci` and `ci-beam.apache.org`, but as I understand, plugin and ci-beam are two totally different things. Maybe something like `beam-jenkins-plugin`may be more informative? WDYT?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [beam] JustineKoa commented on a change in pull request #12042: WIP: Jenkins Plugin

Posted by GitBox <gi...@apache.org>.
JustineKoa commented on a change in pull request #12042:
URL: https://github.com/apache/beam/pull/12042#discussion_r447190465



##########
File path: beam-ci/src/main/resources/io/jenkins/plugins/ExecuteBeamPipelineOnDataflowBuilder/help-buildReleaseOptions.html
##########
@@ -0,0 +1,4 @@
+<div>
+    Include optional build release options. <br/>
+    <b>Maven and Gradle example</b>: -Pdataflow-runner
+</div>

Review comment:
       Hmmm I'm not sure what you mean by this. New lines instead of using ```<br/>```?
   Edit: Got it!




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org