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

[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