StackStorm / orquestaconvert

Converts Mistral workflows into Orchestra workflows
Apache License 2.0
13 stars 7 forks source link

Add support for converting concurrency expressions #19

Closed blag closed 5 years ago

blag commented 5 years ago

This PR corrects how concurrency attributes are converted by converting them as expressions...if they are expressions. If they are not expressions they are not touched.

So this snippet:

vars:
  my_list:
    - one
    - two
    - three
tasks:
  initial_random_task:
    with-items: i in <% $.my_list %>
    concurrency: <% $.my_list.count() %>  # <- YAQL expression

would get (incompletely) converted to this:

vars:
  my_list:
    - one
    - two
    - three
tasks:
  initial_random_task:
    with:
      items: i in <% ctx().my_list %>  # <- correctly uses ctx() for context variable access
      concurrency: <% $.my_list.count() %>  # <- incorrectly still uses $ for context variable access

this PR adds support so the concurrency attribute is treated like an expression and converted correctly:

vars:
  my_list:
    - one
    - two
    - three
tasks:
  initial_random_task:
    with:
      items: i in <% ctx().my_list %>
      concurrency: <% ctx().my_list.count() %>  # <- now uses ctx() for context variable access

I added unit tests for cases where concurrency is an integer, a string that represents an integer, and YAQL and Jinja expressions, as well as end-to-end test files for the same.

I also made a single-line change to orquestaconvert/expressions/__init__.py, to make it slightly more bulletproof: it now tries to convert expressions to strings before passing them to Orquesta for expression type analysis. This should also make it raise a cleaner error message than the previous "TypeError: expected string or buffer" from the re module, and it better indicates the problem and a potential fix.

codecov-io commented 5 years ago

Codecov Report

Merging #19 into master will increase coverage by 0.01%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #19      +/-   ##
==========================================
+ Coverage   98.57%   98.59%   +0.01%     
==========================================
  Files          12       12              
  Lines         561      568       +7     
==========================================
+ Hits          553      560       +7     
  Misses          8        8
Impacted Files Coverage Δ
orquestaconvert/workflows/base.py 100% <100%> (ø) :arrow_up:
orquestaconvert/expressions/__init__.py 100% <100%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update a39bd93...8212130. Read the comment docs.

blag commented 5 years ago

Resolved. I couldn't use Jinja in when: <% succeeded() %> since the script uses YAQL for other fixtures, but the Jinja fixtures now use Jinja as much as they can.

nmaludy commented 5 years ago

LGTM, thanks!