-
Notifications
You must be signed in to change notification settings - Fork 49
Add RunTask.shell task #898
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
@ricardozanini could you share with me a example of arguments? Should be something similar to: document:
dsl: '1.0.1'
namespace: test
name: run-shell-example
version: '0.1.0'
do:
- runShell:
run:
shell:
command: 'kubectl'
arguments:
- 'apply'
- '-f'
- '/k8s'And the output should be: kubectl apply -f /k8sI am assuming that |
|
Great @fjtirado, nice abstractions ;) |
|
@mcruzdev yes, the arguments are a list: https://github.com/serverlessworkflow/specification/blob/main/dsl-reference.md#shell-process Which kind of example do you need? |
Ok, it is all, I think I do not need a example 😄 I had a confusion due to |
You can do: arguments:
- key: valueMaybe in some use cases that would make sense. |
impl/core/src/main/java/io/serverlessworkflow/impl/executors/RunShellExecutor.java
Outdated
Show resolved
Hide resolved
impl/core/src/main/java/io/serverlessworkflow/impl/executors/RunShellExecutor.java
Outdated
Show resolved
Hide resolved
impl/core/src/main/java/io/serverlessworkflow/impl/executors/RunShellExecutor.java
Outdated
Show resolved
Hide resolved
impl/core/src/main/java/io/serverlessworkflow/impl/executors/RunShellExecutor.java
Outdated
Show resolved
Hide resolved
It is not clear to me again @ricardozanini, what is the real command statement in that case? The arguments will be used like an input (like |
| StringBuilder stdout = new StringBuilder(); | ||
| StringBuilder stderr = new StringBuilder(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Althour the override of outputstream is quite imaginative and I like it, I have to mention that is better to use standard library, using
StringWriter outWriter = new StringWriter();
process.getInputStream().transferTo(new OutputStreamWriter(outWriter));
.....
outWriter.toString();
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that the OutputStreamWriter is not possible, but yes, we need a better solution that is not a byte to byte.
EDIT: I mean in the transferTo method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work!
Just minor comments, except the error one.
Signed-off-by: Matheus Cruz <[email protected]>
Context
Implement RunTask.shell task:
Tracking
awaitreturnCloses #892