Skip to content
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

Valid YAQL in With Items Input Fails #247

Open
jamesdreid opened this issue Mar 17, 2022 · 8 comments
Open

Valid YAQL in With Items Input Fails #247

jamesdreid opened this issue Mar 17, 2022 · 8 comments
Labels

Comments

@jamesdreid
Copy link

Creating a workflow that includes a "with-items" task that uses a valid YAQL statement to supply a list to the input of the task fails. It appears that the workflow will not longer properly evaluated the YAQL prior to running the task.

The following task config does not work:

withitem_task:
    with:
      items: <% ctx(dict_obj).keys() %>
    action: core.noop

Using the exact same YAQL statement in the publish statement of a prior task will generate a proper array and works as expected when referenced from the context in the "items" section.

@jamesdreid
Copy link
Author

Sorry, the issue was first identified in our workflows after an upgrade from 3.4.1 to 3.6. I do not know if it was there in 3.5 as we jumped straight to 3.6 on the system where these failing workflows were running.

@nzlosh
Copy link
Contributor

nzlosh commented Feb 20, 2023

After some discussion and investigation with @jamesdreid I was able to identify the items() behaviour has changed between yaql v1.1.3 and yaql v2.0.0

YAQL is currently pinned as yaql>=1.1.0
According to pypi, yaql was updated to 2.0.0 on 12 Jul 2021, which is just after st2 v3.5 and would have been included in the st2 v3.6

@arm4b
Copy link
Member

arm4b commented Feb 22, 2023

@nzlosh Any next steps? Do we need to pin yaql in the requirements to fix it?

@arm4b arm4b added the bug label Feb 22, 2023
@nzlosh
Copy link
Contributor

nzlosh commented Feb 22, 2023

I've looked into this further, this is the commit for yaql v2.0.0

The abstract base classes previously defined in 'collections' were moved
to 'collections.abc' in 3.3. The aliases will be removed in 3.10.

-    return isinstance(obj, collections.Iterator)
+    return isinstance(obj, collections.abc.Iterator)

If we pin to 1.1.3, builds will fail against py3.10 (which will be required to support ubuntu 22.04 and drop ubuntu 18.04). I think we're at the point where we'll need to fix the orquesta code. Which will be related to how it's detecting/handling collections.

@jamesdreid
Copy link
Author

Looks like this PR may have been related to the attempted fix for a similar issue that may need some updates. #191

@jamesdreid
Copy link
Author

Plus it appears that Orquesta may be importing "collections" as well and may need to be updated in a manner similar to the YAQL lib.
https://github.com/userlocalhost/orquesta/blob/4bc557574042a970a2101d15fecf626d0f93a4e7/orquesta/expressions/functions/workflow.py#L15

@cognifloyd
Copy link
Member

Agreed. Let's fix orquesta, not the pin.

@nzlosh
Copy link
Contributor

nzlosh commented Feb 23, 2023

It seems #191 is a partial fix. Testing a few expressions shows the following:

YAQL list ✔️

    action: core.local
    with: <% [1,2] %>
    input:
      cmd: echo <% item() %>

YAQL dict keys

    action: core.local
    with: <% { "a" => 1, "b" => 2 }.keys() %>
    input:
      cmd: echo <% item() %>
'TypeError: The value of "<% { "a" => 1, "b" => 2 }.keys() %>" is not type of list.'

YAQL dict keys using X in format

    action: core.local
    with: keyname in <% { "a" => 1, "b" => 2 }.keys() %>
    input:
      cmd: echo <% item() %>
'TypeError: The value of "<% { "a" => 1, "b" => 2 }.keys() %>" is not type of list.'

YAQL cast dict keys to list ✔️

    action: core.local
    with: <% { "a" => 1, "b" => 2 }.keys().toList() %>
    input:
      cmd: echo <% item() %>

Jinja list

    action: core.local
    with: {{ [1,2] }}
    input:
      cmd: echo <% item() %>

"Failed to load workflow definition because while constructing a mapping
  in "<unicode string>", line 7, column 12
found unacceptable key (unhashable type: 'list')
  in "<unicode string>", line 7, column 14."

Jinja list using X in format ✔️

    action: core.local
    with: keyname in {{ [1,2] }}
    input:
      cmd: echo <% item() %>

stdout: '{keyname: 1}'

Jinja dict keys

    action: core.local
    with: {{ {"a": 1, "b": 2}.keys() }}
    input:
      cmd: echo <% item() %>

"Failed to load workflow definition because while parsing a flow mapping
  in "<unicode string>", line 7, column 12
did not find expected ',' or '}'
  in "<unicode string>", line 7, column 30."

Jinja dict keys using X in format

    action: core.local
    with: keyname in {{ {"a": 1, "b": 2}.keys() }}
    input:
      cmd: echo <% item() %>

"Failed to load workflow definition because mapping values are not allowed in this context
  in "<unicode string>", line 7, column 29."

Jina cast dict keys to list using X in format

    action: core.local
    with: keyname in {{ {"a": 1, "b": 2}.keys() | list }}
    input:
      cmd: echo <% item() %>

"Failed to load workflow definition because mapping values are not allowed in this context
  in "<unicode string>", line 7, column 29."

It's not immediately obvious to me where the root cause of this issue is.

nzlosh added a commit to nzlosh/orquesta that referenced this issue Aug 13, 2023
nzlosh added a commit to nzlosh/orquesta that referenced this issue Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants