feat(core): deprecate old script tasks and stop using them in tests (#…

…1792) * feat(core): deprecate old script tasks and stop using them in tests Only their own tests still use them. But flow tests use them A LOT in the flow YAML definitions! * Update core/src/test/java/io/kestra/core/tasks/flows/TimeoutTest.java Co-authored-by: brian-mulier-p <[email protected]> --------- Co-authored-by: brian-mulier-p <[email protected]>
kestra-io · Jul 21, 2023 · 1884ab4 · 1884ab4
1 parent d1bdeba
commit 1884ab4
Show file tree

Hide file tree

Showing 21 changed files with 98 additions and 53 deletions.
diff --git a/README.md b/README.md
@@ -113,17 +113,17 @@ tasks:
     concurrent: 3
     tasks:
       - id: task1
-        type: io.kestra.core.tasks.scripts.Bash
+        type: io.kestra.plugin.scripts.shell.Commands
         commands:
           - 'echo "running {{task.id}}"'
           - 'sleep 10'
       - id: task2
-        type: io.kestra.core.tasks.scripts.Bash
+        type: io.kestra.plugin.scripts.shell.Commands
         commands:
           - 'echo "running {{task.id}}"'
           - 'sleep 10'
       - id: task3
-        type: io.kestra.core.tasks.scripts.Bash
+        type: io.kestra.plugin.scripts.shell.Commands
         commands:
           - 'echo "running {{task.id}}"'
           - 'sleep 10'

diff --git a/core/src/main/java/io/kestra/core/tasks/scripts/AbstractBash.java b/core/src/main/java/io/kestra/core/tasks/scripts/AbstractBash.java
@@ -33,6 +33,7 @@
 @EqualsAndHashCode
 @Getter
 @NoArgsConstructor
+@Deprecated
 abstract public class AbstractBash extends Task {
     @Builder.Default
     @Schema(

diff --git a/core/src/main/java/io/kestra/core/tasks/scripts/AbstractLogThread.java b/core/src/main/java/io/kestra/core/tasks/scripts/AbstractLogThread.java
@@ -8,6 +8,7 @@
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
+@Deprecated
 public abstract class AbstractLogThread extends Thread {
     private final InputStream inputStream;
     private int logsCount = 0;

diff --git a/core/src/main/java/io/kestra/core/tasks/scripts/AbstractPython.java b/core/src/main/java/io/kestra/core/tasks/scripts/AbstractPython.java
@@ -25,6 +25,7 @@
 @Getter
 @NoArgsConstructor
 @Slf4j
+@Deprecated
 public abstract class AbstractPython extends AbstractBash {
     @Builder.Default
     @Schema(

diff --git a/core/src/main/java/io/kestra/core/tasks/scripts/Bash.java b/core/src/main/java/io/kestra/core/tasks/scripts/Bash.java
@@ -25,7 +25,9 @@
 @Getter
 @NoArgsConstructor
 @Schema(
-    title = "Execute a Bash script, command or set of commands."
+    title = "Execute a Bash script, command or set of commands.",
+    description = "This task is deprecated, please use the io.kestra.plugin.scripts.shell.Script or io.kestra.plugin.scripts.shell.Commands task instead.",
+    deprecated = true
 )
 @Plugin(
     examples = {
@@ -103,6 +105,7 @@
         )
     }
 )
+@Deprecated
 public class Bash extends AbstractBash implements RunnableTask<ScriptOutput> {
     @Schema(
         title = "The commands to run",

diff --git a/core/src/main/java/io/kestra/core/tasks/scripts/BashService.java b/core/src/main/java/io/kestra/core/tasks/scripts/BashService.java
@@ -22,6 +22,7 @@
 
 import static io.kestra.core.utils.Rethrow.throwConsumer;
 
+@Deprecated
 abstract public class BashService {
     protected static final ObjectMapper MAPPER = JacksonMapper.ofJson();
     private static final Pattern PATTERN = Pattern.compile("^::(\\{.*})::$");

diff --git a/core/src/main/java/io/kestra/core/tasks/scripts/Node.java b/core/src/main/java/io/kestra/core/tasks/scripts/Node.java
@@ -27,7 +27,8 @@
 @NoArgsConstructor
 @Schema(
     title = "Execute a Node.js script",
-    description = "With the Node task, you can execute a full javascript script.\n" +
+    description = "This task is deprecated, please use the io.kestra.plugin.scripts.node.Script or io.kestra.plugin.scripts.node.Commands task instead.\n\n" +
+        "With the Node task, you can execute a full javascript script.\n" +
         "The task will create a temporary folder for each tasks and allows to install some npm packages defined in an optional `package.json` file.\n" +
         "\n" +
         "By convention, you need to define at least a `main.js` files in `inputFiles` that will be the script used.\n" +
@@ -41,7 +42,8 @@
         "Kestra.counter('count', 1, {tag1: 'i', tag2: 'win'});\n" +
         "Kestra.timer('timer1', (callback) => { setTimeout(callback, 1000) }, {tag1: 'i', tag2: 'lost'});\n" +
         "Kestra.timer('timer2', 2.12, {tag1: 'i', tag2: 'destroy'});\n" +
-        "```"
+        "```",
+    deprecated = true
 )
 @Plugin(
     examples = {
@@ -94,6 +96,7 @@
         )
     }
 )
+@Deprecated
 public class Node extends AbstractBash implements RunnableTask<ScriptOutput> {
     @Builder.Default
     @Schema(

diff --git a/core/src/main/java/io/kestra/core/tasks/scripts/Python.java b/core/src/main/java/io/kestra/core/tasks/scripts/Python.java
@@ -27,7 +27,8 @@
 @NoArgsConstructor
 @Schema(
     title = "Execute a Python script",
-    description = "With the Python task, you can execute a full Python script.\n" +
+    description = "This task is deprecated, please use the io.kestra.plugin.scripts.python.Script or io.kestra.plugin.scripts.python.Commands task instead.\n\n" +
+        "With the Python task, you can execute a full Python script.\n" +
         "The task will create a fresh `virtualenv` for every tasks and allows to install some Python package define in `requirements` property.\n" +
         "\n" +
         "By convention, you need to define at least a `main.py` files in `inputFiles` that will be the script used.\n" +
@@ -43,7 +44,8 @@
         "Kestra.counter('count', 1, {'tag1': 'i', 'tag2': 'win'})\n" +
         "Kestra.timer('timer1', lambda: time.sleep(1), {'tag1': 'i', 'tag2': 'lost'})\n" +
         "Kestra.timer('timer2', 2.12, {'tag1': 'i', 'tag2': 'destroy'})\n" +
-        "```"
+        "```",
+    deprecated = true
 )
 @Plugin(
     examples = {
@@ -84,6 +86,7 @@
     }
 )
 @Slf4j
+@Deprecated
 public class Python extends AbstractPython implements RunnableTask<ScriptOutput> {
     @Schema(
         title = "The commands to run",

diff --git a/core/src/main/java/io/kestra/core/tasks/scripts/RunResult.java b/core/src/main/java/io/kestra/core/tasks/scripts/RunResult.java
@@ -3,6 +3,7 @@
 import lombok.AllArgsConstructor;
 import lombok.Getter;
 
+@Deprecated
 @AllArgsConstructor
 @Getter
 public class RunResult {

diff --git a/core/src/main/java/io/kestra/core/tasks/scripts/ScriptOutput.java b/core/src/main/java/io/kestra/core/tasks/scripts/ScriptOutput.java
@@ -15,6 +15,7 @@
 
 @Builder
 @Getter
+@Deprecated
 public class ScriptOutput implements io.kestra.core.models.tasks.Output {
     @Schema(
         title = "The value extract from output of the commands"

diff --git a/core/src/main/java/io/kestra/core/tasks/scripts/runners/DockerScriptRunner.java b/core/src/main/java/io/kestra/core/tasks/scripts/runners/DockerScriptRunner.java
@@ -38,6 +38,7 @@
 
 import static io.kestra.core.utils.Rethrow.throwFunction;
 
+@Deprecated
 public class DockerScriptRunner implements ScriptRunnerInterface {
     private static final ReadableBytesTypeConverter READABLE_BYTES_TYPE_CONVERTER = new ReadableBytesTypeConverter();
 

diff --git a/core/src/main/java/io/kestra/core/tasks/scripts/runners/ProcessBuilderScriptRunner.java b/core/src/main/java/io/kestra/core/tasks/scripts/runners/ProcessBuilderScriptRunner.java
@@ -14,6 +14,7 @@
 
 import static io.kestra.core.utils.Rethrow.throwFunction;
 
+@Deprecated
 public class ProcessBuilderScriptRunner implements ScriptRunnerInterface {
     public RunResult run(
         AbstractBash abstractBash,

diff --git a/core/src/main/java/io/kestra/core/tasks/scripts/runners/ScriptRunnerInterface.java b/core/src/main/java/io/kestra/core/tasks/scripts/runners/ScriptRunnerInterface.java
@@ -9,6 +9,7 @@
 import java.util.List;
 import java.util.Map;
 
+@Deprecated
 public interface ScriptRunnerInterface {
     RunResult run(
         AbstractBash abstractBash,

diff --git a/core/src/test/java/io/kestra/core/docs/DocumentationGeneratorTest.java b/core/src/test/java/io/kestra/core/docs/DocumentationGeneratorTest.java
@@ -5,9 +5,8 @@
 import io.kestra.core.plugins.RegisteredPlugin;
 import io.kestra.core.tasks.debugs.Echo;
 import io.kestra.core.tasks.debugs.Return;
+import io.kestra.core.tasks.flows.Dag;
 import io.kestra.core.tasks.flows.Flow;
-import io.kestra.core.tasks.log.Log;
-import io.kestra.core.tasks.scripts.Bash;
 import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
 import jakarta.inject.Inject;
 import org.junit.jupiter.api.Test;
@@ -18,9 +17,7 @@
 import java.nio.file.Paths;
 import java.util.Arrays;
 import java.util.List;
-import java.util.Map;
 import java.util.Objects;
-import java.util.stream.Stream;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.*;
@@ -53,26 +50,24 @@ void tasks() throws URISyntaxException, IOException {
 
     @SuppressWarnings({"rawtypes", "unchecked"})
     @Test
-    void bash() throws IOException {
+    void dag() throws IOException {
         PluginScanner pluginScanner = new PluginScanner(ClassPluginDocumentationTest.class.getClassLoader());
         RegisteredPlugin scan = pluginScanner.scan();
-        Class bash = scan.findClass(Bash.class.getName()).orElseThrow();
+        Class dag = scan.findClass(Dag.class.getName()).orElseThrow();
 
-        ClassPluginDocumentation<? extends Task> doc = ClassPluginDocumentation.of(jsonSchemaGenerator, scan, bash, Task.class);
+        ClassPluginDocumentation<? extends Task> doc = ClassPluginDocumentation.of(jsonSchemaGenerator, scan, dag, Task.class);
 
         String render = DocumentationGenerator.render(doc);
 
-        assertThat(render, containsString("Bash"));
+        assertThat(render, containsString("Dag"));
         assertThat(render, containsString("**Required:** ✔️"));
 
-        assertThat(render, containsString("`exitOnFailed`"));
+        assertThat(render, containsString("`concurrent`"));
 
         int propertiesIndex = render.indexOf("Properties");
-        int outputsIndex = render.indexOf("Outputs");
         int definitionsIndex = render.indexOf("Definitions");
 
-        assertRequiredPropsAreFirst(render.substring(propertiesIndex, outputsIndex));
-        assertRequiredPropsAreFirst(render.substring(outputsIndex, definitionsIndex));
+        assertRequiredPropsAreFirst(render.substring(propertiesIndex, definitionsIndex));
 
         String definitionsDoc = render.substring(definitionsIndex);
         Arrays.stream(definitionsDoc.split("[^#]### "))

diff --git a/core/src/test/java/io/kestra/core/docs/JsonSchemaGeneratorTest.java b/core/src/test/java/io/kestra/core/docs/JsonSchemaGeneratorTest.java
@@ -13,8 +13,7 @@
 import io.kestra.core.runners.RunContext;
 import io.kestra.core.tasks.debugs.Echo;
 import io.kestra.core.tasks.debugs.Return;
-import io.kestra.core.tasks.log.Log;
-import io.kestra.core.tasks.scripts.Bash;
+import io.kestra.core.tasks.flows.Dag;
 import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
 import io.swagger.v3.oas.annotations.media.Schema;
 import jakarta.inject.Inject;
@@ -127,17 +126,16 @@ void trigger() throws URISyntaxException {
 
     @SuppressWarnings("unchecked")
     @Test
-    void bash() throws URISyntaxException {
+    void dag() throws URISyntaxException {
         Helpers.runApplicationContext((applicationContext) -> {
             JsonSchemaGenerator jsonSchemaGenerator = applicationContext.getBean(JsonSchemaGenerator.class);
 
-            Map<String, Object> generate = jsonSchemaGenerator.schemas(Bash.class);
+            Map<String, Object> generate = jsonSchemaGenerator.schemas(Dag.class);
 
             var definitions = (Map<String, Map<String, Object>>) generate.get("definitions");
 
-            var bash = definitions.get("io.kestra.core.tasks.scripts.Bash-1");
-            assertThat((List<String>) bash.get("required"), not(contains("exitOnFailed")));
-            assertThat((List<String>) bash.get("required"), not(contains("interpreter")));
+            var dag = definitions.get("io.kestra.core.tasks.flows.Dag-1");
+            assertThat((List<String>) dag.get("required"), not(contains("errors")));
         });
     }
 

diff --git a/core/src/test/java/io/kestra/core/models/flows/FlowWithSourceTest.java b/core/src/test/java/io/kestra/core/models/flows/FlowWithSourceTest.java
@@ -4,7 +4,7 @@
 import io.kestra.core.models.triggers.types.Schedule;
 import io.kestra.core.serializers.JacksonMapper;
 import io.kestra.core.tasks.debugs.Return;
-import io.kestra.core.tasks.scripts.Bash;
+import io.kestra.core.tasks.log.Log;
 import io.kestra.core.utils.IdUtils;
 import org.junit.jupiter.api.Test;
 
@@ -46,10 +46,10 @@ void scalar() throws JsonProcessingException {
             .id(IdUtils.create())
             .namespace("io.kestra.unittest")
             .tasks(List.of(
-                Bash.builder()
+                Log.builder()
                     .id(IdUtils.create())
-                    .type(Return.class.getName())
-                    .commands(new String[]{"timeout 1 bash -c 'cat < /dev/null > /dev/tcp/{{ inputs.host }}/{{ inputs.port }}'", "echo $?"})
+                    .type(Log.class.getName())
+                    .message("Hello World")
                     .build()
             ))
             .triggers(List.of(Schedule.builder().id("schedule").cron("0 1 9 * * *").build()));
@@ -60,8 +60,7 @@ void scalar() throws JsonProcessingException {
 
         String source = flow.getSource();
 
-        assertThat(source, containsString("  - \"timeout "));
-        assertThat(source, containsString("  - echo"));
+        assertThat(source, containsString("message: Hello World"));
         assertThat(source, containsString("  cron: 0 1 9 * * *"));
     }
 }
diff --git a/core/src/test/java/io/kestra/core/repositories/AbstractFlowRepositoryTest.java b/core/src/test/java/io/kestra/core/repositories/AbstractFlowRepositoryTest.java
@@ -16,7 +16,7 @@
 import io.kestra.core.serializers.JacksonMapper;
 import io.kestra.core.services.TaskDefaultService;
 import io.kestra.core.tasks.debugs.Return;
-import io.kestra.core.tasks.scripts.Bash;
+import io.kestra.core.tasks.log.Log;
 import io.kestra.core.utils.Await;
 import io.kestra.core.utils.IdUtils;
 import io.kestra.core.utils.TestsUtils;
@@ -131,10 +131,10 @@ protected void revision() throws JsonProcessingException {
             .id(flowId)
             .namespace("io.kestra.unittest")
             .tasks(Collections.singletonList(
-                Bash.builder()
-                    .id("id")
-                    .type(Bash.class.getName())
-                    .commands(Collections.singletonList("echo 1").toArray(new String[0]))
+                Log.builder()
+                    .id(IdUtils.create())
+                    .type(Log.class.getName())
+                    .message("Hello World")
                     .build()
             ))
             .inputs(ImmutableList.of(StringInput.builder().type(Input.Type.STRING).name("b").build()))

diff --git a/...ra/core/runners/WorkingDirectoryTest.java → ...va/io/kestra/core/runners/WorkerTest.java b/...ra/core/runners/WorkingDirectoryTest.java → ...va/io/kestra/core/runners/WorkerTest.java
@@ -4,6 +4,7 @@
 import io.kestra.core.models.executions.LogEntry;
 import io.kestra.core.tasks.flows.Pause;
 import io.kestra.core.tasks.flows.WorkingDirectory;
+import io.kestra.core.tasks.test.Sleep;
 import io.micronaut.context.ApplicationContext;
 import io.micronaut.test.extensions.junit5.annotation.MicronautTest;
 import org.junit.jupiter.api.Test;
@@ -15,7 +16,6 @@
 import io.kestra.core.models.tasks.ResolvedTask;
 import io.kestra.core.queues.QueueFactoryInterface;
 import io.kestra.core.queues.QueueInterface;
-import io.kestra.core.tasks.scripts.Bash;
 import io.kestra.core.utils.Await;
 import io.kestra.core.utils.IdUtils;
 import io.kestra.core.utils.TestsUtils;
@@ -34,7 +34,7 @@
 import static org.hamcrest.Matchers.nullValue;
 
 @MicronautTest
-class WorkingDirectoryTest {
+class WorkerTest {
     @Inject
     ApplicationContext applicationContext;
 
@@ -65,7 +65,7 @@ void success() throws TimeoutException {
         AtomicReference<WorkerTaskResult> workerTaskResult = new AtomicReference<>(null);
         workerTaskResultQueue.receive(workerTaskResult::set);
 
-        workerTaskQueue.emit(workerTask("1"));
+        workerTaskQueue.emit(workerTask(1000));
 
         Await.until(
             () -> workerTaskResult.get() != null && workerTaskResult.get().getTaskRun().getState().isTerminated(),
@@ -140,14 +140,14 @@ void killed() throws InterruptedException, TimeoutException {
         List<WorkerTaskResult> workerTaskResult = new ArrayList<>();
         workerTaskResultQueue.receive(workerTaskResult::add);
 
-        WorkerTask workerTask = workerTask("999");
+        WorkerTask workerTask = workerTask(999000);
 
         workerTaskQueue.emit(workerTask);
         workerTaskQueue.emit(workerTask);
         workerTaskQueue.emit(workerTask);
         workerTaskQueue.emit(workerTask);
 
-        WorkerTask notKilled = workerTask("2");
+        WorkerTask notKilled = workerTask(2000);
         workerTaskQueue.emit(notKilled);
 
         Thread.sleep(500);
@@ -179,11 +179,11 @@ void killed() throws InterruptedException, TimeoutException {
         assertThat(logs.stream().filter(logEntry -> logEntry.getMessage().equals("3")).count(), is(0L));
     }
 
-    private WorkerTask workerTask(String sleep) {
-        Bash bash = Bash.builder()
-            .type(Bash.class.getName())
+    private WorkerTask workerTask(long sleepDuration) {
+        Sleep bash = Sleep.builder()
+            .type(Sleep.class.getName())
             .id("unit-test")
-            .commands(new String[]{"for i in $(seq 1 " + sleep + "); do echo $i; sleep 1; done"})
+            .duration(sleepDuration)
             .build();
 
         Flow flow = Flow.builder()
-Original file line number
+Diff line change
@@ Expand Up / @@ -15,6 +15,7 @@ @@
     @Builder
     @Getter
+    @Deprecated
     public class ScriptOutput implements io.kestra.core.models.tasks.Output {
         @Schema(
             title = "The value extract from output of the commands"
@@ Expand Down @@