Skip to content

Commit

Permalink
[Improvement][Test] Give an example on replacing Powermock with Mocki…
Browse files Browse the repository at this point in the history 
…to (#11588)

* Give an example on replacing powermock with mockito

* Remove redundant comments

* Refactoring related UTs for better readability.
  • Loading branch information
@EricGao888
EricGao888 authored Sep 20, 2022
1 parent b3bbf69 commit e1b55db
Show file tree
Hide file tree
Showing 7 changed files with 143 additions and 216 deletions.
52 changes: 11 additions & 41 deletions ...yter/src/main/java/org/apache/dolphinscheduler/plugin/task/jupyter/JupyterParameters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,70 +17,38 @@

package org.apache.dolphinscheduler.plugin.task.jupyter;

import lombok.Getter;
import lombok.Setter;
import lombok.ToString;
import org.apache.dolphinscheduler.plugin.task.api.model.ResourceInfo;
import org.apache.dolphinscheduler.plugin.task.api.parameters.AbstractParameters;
import org.apache.dolphinscheduler.spi.utils.StringUtils;

import java.util.List;

/**
* jupyter parameters
*/
@Getter
@Setter
@ToString
import lombok.Data;

@Data
public class JupyterParameters extends AbstractParameters {

/**
* conda env name
*/
private String condaEnvName;

/**
* input note path
*/
private String inputNotePath;

/**
* output note path
*/
private String outputNotePath;

/**
* parameters to pass into jupyter note cells
*/
// parameters to pass into jupyter note cells
private String parameters;

/**
* jupyter kernel
*/
private String kernel;

/**
* the execution engine name to use in evaluating the notebook
*/
// the execution engine name to use in evaluating the notebook
private String engine;

/**
* time in seconds to wait for each cell before failing execution (default: forever)
*/
// time in seconds to wait for each cell before failing execution (default: forever)
private String executionTimeout;

/**
* time in seconds to wait for kernel to start
*/
// time in seconds to wait for kernel to start
private String startTimeout;

/**
* other arguments
*/
private String others;

/**
* resource list
*/
private List<ResourceInfo> resourceList;

@Override
Expand All @@ -90,7 +58,9 @@ public List<ResourceInfo> getResourceFilesList() {

@Override
public boolean checkParameters() {
return condaEnvName != null && inputNotePath != null && outputNotePath != null;
return StringUtils.isNotEmpty(condaEnvName) &&
StringUtils.isNotEmpty(inputNotePath) &&
StringUtils.isNotEmpty(outputNotePath);
}

}
40 changes: 9 additions & 31 deletions ...sk-jupyter/src/main/java/org/apache/dolphinscheduler/plugin/task/jupyter/JupyterTask.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,8 @@

public class JupyterTask extends AbstractRemoteTask {

/**
* jupyter parameters
*/
private JupyterParameters jupyterParameters;

/**
* taskExecutionContext
*/
private TaskExecutionContext taskExecutionContext;

private ShellCommandExecutor shellCommandExecutor;
Expand Down Expand Up @@ -88,7 +82,6 @@ public void init() {
@Override
public void handle(TaskCallBack taskCallBack) throws TaskException {
try {
// SHELL task exit code
TaskResponse response = shellCommandExecutor.run(buildCommand());
setExitStatusCode(response.getExitStatusCode());
setAppIds(String.join(TaskConstants.COMMA, getApplicationIds()));
Expand Down Expand Up @@ -116,16 +109,12 @@ public void trackApplicationStatus() throws TaskException {
}

/**
* create command
*
* @return command
* command will be like: papermill [OPTIONS] NOTEBOOK_PATH [OUTPUT_PATH]
*/
protected String buildCommand() throws IOException {
/**
* papermill [OPTIONS] NOTEBOOK_PATH [OUTPUT_PATH]
*/

List<String> args = new ArrayList<>();
final String condaPath = PropertyUtils.getString(TaskConstants.CONDA_PATH);
final String condaPath = readCondaPath();
final String timestamp = DateUtils.getTimestampString();
String condaEnvName = jupyterParameters.getCondaEnvName();
if (condaEnvName.endsWith(JupyterConstants.TXT_SUFFIX)) {
Expand All @@ -149,11 +138,7 @@ protected String buildCommand() throws IOException {
args.add(JupyterConstants.PAPERMILL);
args.add(jupyterParameters.getInputNotePath());
args.add(jupyterParameters.getOutputNotePath());

// populate jupyter parameterization
args.addAll(populateJupyterParameterization());

// populate jupyter options
args.addAll(populateJupyterOptions());

// remove tmp conda env, if created from requirements.txt
Expand All @@ -164,7 +149,6 @@ protected String buildCommand() throws IOException {

// replace placeholder, and combining local and global parameters
Map<String, Property> paramsMap = taskExecutionContext.getPrepareParamsMap();

String command = ParameterUtils
.convertParameterPlaceholders(String.join(" ", args), ParamUtils.convert(paramsMap));

Expand All @@ -173,12 +157,11 @@ protected String buildCommand() throws IOException {
return command;
}

/**
* build jupyter parameterization
*
* @return argument list
*/
private List<String> populateJupyterParameterization() throws IOException {
protected String readCondaPath() {
return PropertyUtils.getString(TaskConstants.CONDA_PATH);
}

protected List<String> populateJupyterParameterization() throws IOException {
List<String> args = new ArrayList<>();
String parameters = jupyterParameters.getParameters();
if (StringUtils.isNotEmpty(parameters)) {
Expand All @@ -200,12 +183,7 @@ private List<String> populateJupyterParameterization() throws IOException {
return args;
}

/**
* build jupyter options
*
* @return argument list
*/
private List<String> populateJupyterOptions() {
protected List<String> populateJupyterOptions() {
List<String> args = new ArrayList<>();
String kernel = jupyterParameters.getKernel();
if (StringUtils.isNotEmpty(kernel)) {
Expand Down
Loading

0 comments on commit e1b55db

Please sign in to comment.