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:
us...@infra.apache.org


Reply via email to