Orquesta and st2kv() bug/regression

[emptywee]

SUMMARY

Orquesta st2kv() behavior changed drastically, making it hard to migrate from Mistral engine.

1. It now throws exception if the specified key doesn't exist in the datastore (previously it would return null). Exceptions seem to be impossible to handle within YAQL/Jinja expressions.
2. Even if the key exists, but has an empty string value, it still throws an exception. See https://github.com/StackStorm/st2/blob/master/contrib/runners/orquesta_runner/orquesta_functions/st2kv.py#L48 - this is not the best option chosen for the if condition, in my humble opinion.

ISSUE TYPE

• Bug Report

STACKSTORM VERSION

Paste the output of st2 --version:

$ st2 --version
st2 3.0.0, on Python 2.7.10

OS / ENVIRONMENT / INSTALL METHOD

CentOS 6, custom install.

STEPS TO REPRODUCE

Meta:

---
name: wf_orchestra_test
pack: playground
description: Test
runner_type: orquesta
entry_point: workflows/wf_orchestra_test.yaml
enabled: true
parameters:
  name:
    required: true
    type: string

Workflow:

---
version: '1.0'
input: 
  - name
tasks:
  task1:
    action: core.noop
    next:
      - when: <% succeeded() %>
        publish:
          - val: <% st2kv('system.' + ctx().name) %>

EXPECTED RESULTS

null published to the val variable

ACTUAL RESULTS

$ st2 run playground.wf_orchestra_test name=ttt
.
id: 5cdc88702db7aa204ea689d9
action.ref: playground.wf_orchestra_test
parameters: 
  name: ttt
status: failed
start_timestamp: Wed, 15 May 2019 16:45:20 CDT
end_timestamp: Wed, 15 May 2019 16:45:21 CDT
result: 
  errors:
  - message: 'YaqlEvaluationException: Unable to evaluate expression ''<% st2kv(''system.'' + ctx().name) %>''. ExpressionEvaluationException: Key system.ttt does not exist in StackStorm datastore.'
    route: 0
    task_id: task1
    task_transition_id: noop__t0
    type: error
  output: null

And to illustrate item 2:

$ st2 key set ttt ''
+------------------+--------------+
| Property         | Value        |
+------------------+--------------+
| name             | ttt          |
| value            |              |
| scope            | st2kv.system |
| expire_timestamp |              |
+------------------+--------------+
$ st2 key get ttt
+------------------+--------------+
| Property         | Value        |
+------------------+--------------+
| name             | ttt          |
| value            |              |
| secret           | False        |
| encrypted        | False        |
| scope            | st2kv.system |
| expire_timestamp |              |
+------------------+--------------+
$ st2 run playground.wf_orchestra_test name=ttt
.
id: 5cdc88942db7aa204ea689dd
action.ref: playground.wf_orchestra_test
parameters: 
  name: ttt
status: failed
start_timestamp: Wed, 15 May 2019 16:45:56 CDT
end_timestamp: Wed, 15 May 2019 16:45:57 CDT
result: 
  errors:
  - message: 'YaqlEvaluationException: Unable to evaluate expression ''<% st2kv(''system.'' + ctx().name) %>''. ExpressionEvaluationException: Key system.ttt does not exist in StackStorm datastore.'
    route: 0
    task_id: task1
    task_transition_id: noop__t0
    type: error
  output: null

[jinpingh]

Mistral and orquesta have different implementation. Mistral calls st2 REST api to query key value, if API call returns OK, key value will be returned from response message, for this case, is null. Orquesta query database directly and check return value after. If no record returns, exception is raised.

[jinpingh]

Tried out both mistral and orquesta for invalid key, empty key and key with null value. They have different behaviors. Two issues:

1. For mistral, if key is invalid, exception should be caught. Right now since HTTP returns 201, https://github.com/StackStorm/st2mistral/blob/master/st2mistral/functions/stackstorm.py#L82 exception is not raised.
2. For orquesta, if key is empty, it should return empty string for key, Current code will raise YaqlEvaluationException and with `Key xxxx does not exist in StackStorm datastore.' error message.

[emptywee]

I don't know if it counts, but I'd prefer to get null if key doesn't exist, or at least have an option in st2kv call to not raise exceptions, since it's impossible to handle them within yaql or Jinja expressions.

[m4dcoder]

If key exists and the value is null or empty string, we will fix to return null and empty string and not raise exception. If key does not exists, we will raise an exception. The fact that the mistral counterpart did not raise an exception is because it is a bug. I believe the st2kv should raise an exception if key does not exists so the user knows, especially when they expect it to be there. We can add a default arg in the st2kv function if the user wants to silent the exception and just return some default value.

[emptywee]

@m4dcoder that'll work for me and, I believe, for others as well!

[jinpingh]

Opened #https://github.com/StackStorm/st2mistral/issues/39 for mistral issue.

[emptywee]

Woot-woot! @m4dcoder good job! Thanks bunch!