Transform mistral workbook definition #964
Conversation
|
The DSL transform is temporary until ActionProvider is implemented in Mistral. I punt on DSL validation in this iteration. I did have to eval inline action parameters to transform the workbook properly. To be consistent with the evaluation, I copied two utility functions from Mistral to avoid having to import and depend on the entire mistral module. |
There was a problem hiding this comment.
I am a complete regex n00b so I have no clue what this is doing. Can you add some docs or comments in the method to describe.
There was a problem hiding this comment.
I copied this from Mistral @ https://github.com/stackforge/mistral/blob/master/mistral/workbook/base.py#L23. It's used to evaluate inline action parameters (i.e. action: core.local cmd={$.cmd}). The regex could have been written better. But I would fix it at the Mistral side. I'm just trying to keep the evaluation consistent here.
|
I get the idea that a transform is required although I don't understand the transform. Perhaps it is not worth spending time on the details since you guys expect to deprecate the transformation anyway once the ActionProvider is put in place? |
There was a problem hiding this comment.
Why does the impl here and in st2actions/st2actions/runners/mistral/utils.py look so similar? Perhaps you can call into the utils from here?
Also, what is the purpose of this action? Could the actions have been a separate PR or at least a separate commit?
Btw, we typically put sandbox action in st2incubator now.
There was a problem hiding this comment.
I mean that's what he does.
from st2actions.runners.mistral import utils
But I agree this could have been a separate PR. I was very confused too.
EDIT: I see what you mean. utils.transform_definition() could have been used.
There was a problem hiding this comment.
I'll move this to the st2-incubator. I put this here because the mistral pack was placed here first by lakshmi. I can't use utils.transform_definition() because reading from the Action model is different. The action here uses the st2client whereas the one in utils uses the st2common persistence directly.
|
Yeah as long as this transform works, I am willing to assume the details as a blackbox. There are tests. So we are good. That damn regex made it to our code :). |
|
Hmm ... perhaps it is better if @lakshmi-kannan or @dzimine take a look at this PR. |
|
I think the PR could have been split in the following way:
|
|
Sure it would have been nice with #3. But it's inconsistent with your early comments before that st2 action should not have access to st2 DB directly. |
|
#3 could be create a mistral action. |
|
I'm not sure what you mean. But maybe you can patch afterward. |
The utility function takes a mistral workbook written in native DSL and
wraps st2 action with the st2.action proxy.
Before Transform:
```
name: 'examples.mistral-basic'
version: '2.0'
description: 'Basic mistral workflow example'
workflows:
demo:
type: direct
input:
- cmd
tasks:
run-cmd:
action: core.local cmd={$.cmd}
publish:
stdout: $.stdout
stderr: $.stderr
```
After Transform:
```
name: 'examples.mistral-basic'
version: '2.0'
description: 'Basic mistral workflow example'
workflows:
demo:
type: direct
input:
- cmd
tasks:
run-cmd:
action: st2.action
input:
ref: core.local
parameters:
cmd: $.cmd
publish:
stdout: $.stdout
stderr: $.stderr
```
Allow user to define workbook in native DSL without the st2.action proxy. Before action execution, st2 transforms the workbook definition and adds the st2.action proxy to appropriate actions before creating and updating the workbook in Mistral.
m4dcoder
force-pushed
the
STORM-884/mistral_dsl_transformer
branch
from
b0ce72a to
b522670
Compare
|
Split to multiple commits and added comments for _eval_inline_params. |
Add st2-mistral pack with an action to test the DSL transformation. The transformation takes a native DSL and wraps st2 action with the st2.action proxy. Please refer to PR StackStorm/st2#964 for details of the DSL transformation.
…rmer Transform mistral workbook definition
There was a problem hiding this comment.
It would be nice to make this worklfow actually working against 'mock' actions (make them part of example), run this workflow as part of the test, and literanlinclude it in the docs.
|
LGTM. |
Allow user to define workbook in native DSL without the st2.action
proxy. Before action execution, st2 transforms the workbook definition
and adds the st2.action proxy to appropriate actions before creating and
updating the workbook in Mistral. A st2-mistral pack with an action to
test the transform is included.