Skip to content

Conversation

@winem
Copy link
Contributor

@winem winem commented Jan 12, 2020

This is an example for an Orquesta workflow using the core.ask pack to query multiple parameters. I will also create a PR in the st2docs repository to include this into the documentation.

@pull-request-size pull-request-size bot added the size/M PR that changes 30-99 lines. Good size to review. label Jan 12, 2020
@claassistantio
Copy link

claassistantio commented Jan 12, 2020

CLA assistant check
All committers have signed the CLA.

@winem
Copy link
Contributor Author

winem commented Jan 13, 2020

As far as I can tell the failed Travis CI build is not caused by the new example and rather an issue with the infrastructure. Please let me know if I'm wrong and there is anything I can do to fix the issue.

@arm4b arm4b requested a review from m4dcoder January 14, 2020 13:52
@arm4b
Copy link
Member

arm4b commented Jan 14, 2020

@winem Thanks for the contribution! This looks like an useful example 👍

@m4dcoder Can you please take a look if that workflow looks good to you or there are any recommendations?

@m4dcoder m4dcoder added this to the 3.2.0 milestone Jan 14, 2020
Copy link
Contributor

@m4dcoder m4dcoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just something minor that need to be addressed.

extra_output:
type: string
description: "Your message to echo next if you approve to continue:"
default: "None"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is minor. But you can just use None as in NoneType instead of literal "None" which is a string.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will change that as I actually prefer it. I just didn't know from the docs if that's supported, thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This failed but I'm not sure if this is intended or a bug.

extra_output does not appear in the response object (e.g. "response":{"approved":true,"department_id":41411}) even if a default is defined. So any check on task(get_approval).result.response.extra_output fails with "YaqlEvaluationException: Unable to resolve key 'extra_output' in expression ...

default: "None"
ttl: 60
next:
- when: <% task(get_approval).result.response.extra_output != 'None' and task(get_approval).result.response.approved = true %>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you use NoneType, I believe the check would be task(get_approval).result.response.extra_output != null

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will test this latest tomorrow and come back with a working solution.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test failed. Only boolean, string and names are supported types. I used .containsKey() for this check in the updated code.

- custom_output: <% task(get_approval).result.response.extra_output %>
- approver_department_id: <% task(get_approval).result.response.department_id %>
do: echo_extra_message
- when: <% task(get_approval).result.response.approved = true %>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This condition will be triggered regardless if extra_output is None. So in the case where extra_output is not None, both the echo_extra_message and finish tasks will be triggered simultaneously. Is this your intention? If not, then the condition here should be <% task(get_approval).result.response.extra_output = 'None' and task(get_approval).result.response.approved = true %>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I may have a misunderstanding here. So, my assumption was that the different when conditions are evaluated in their order. As soon as one condition matches the evaluation of the remaining ones will be skipped.

So my idea was:

  • if we have the approval AND a custom message -> echo_extra_message which will trigger the finishing action (the remaining two conditions won't be checked)
  • if stackstorm proceeds to the 2nd condition we know that we got the approval but no extra message (otherwise the condition above would have matched)

Is this assumption wrong? I'll also do some more tests tomorrow to see what I missed during my recent tests.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, your assumption here is not accurate. The transition is evaluated in order but will be triggered if the condition is met. So in this example, multiple task transitions can be triggered.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, got it. Somehow, I get another error when I do not provide any extra_output:
"message": "YaqlEvaluationException: Unable to resolve key 'extra_output' in expression '<% task(get_approval).result.response.extra_output != 'None' and task(get_approval).result.response.approved = true %>' from context.",

This makes sense because the response sent by the browser contains just "response":{"approved":true,"department_id":41411}. So the default is not part of the response object.

I also tried extra_output in task(get_approval.result.response but this seems not to be supported. I will check after lunch how to proceed here. Might be a YAQL expression that's needed here.

Copy link
Contributor Author

@winem winem Jan 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, was able to reproduce the issue and see the behaviour you described. PR will be updated shortly.

Edit: Took care of all your comments and pdated the PR.

Copy link
Contributor

@m4dcoder m4dcoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement size/M PR that changes 30-99 lines. Good size to review. workflows: orquesta

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants