Skip to content

Commit

Permalink
fix(loop): prevent reference to a looped task output artifact
Browse files Browse the repository at this point in the history
Users must now handle looped task artifact aggregation themselves 👷‍

pollination/queenbee-argo#59
  • Loading branch information
AntoineDao committed Aug 27, 2020
1 parent 7112f21 commit 0e99e92
Show file tree
Hide file tree
Showing 10 changed files with 85 additions and 58 deletions.
6 changes: 6 additions & 0 deletions queenbee/recipe/dag.py
Original file line number Diff line number Diff line change
Expand Up @@ -626,6 +626,12 @@ def find_task_output(
task = filtered_tasks[0]

if isinstance(reference, TaskArtifactReference):
if task.loop is not None:
raise ValueError(
'Cannot refer to outputs from a looped task.'
'You must perform your own aggregation and then refer to '
'a hard coded folder path.'
)
return task.outputs.artifact_by_name(reference.variable)
elif isinstance(reference, TaskParameterReference):
return task.outputs.parameter_by_name(reference.variable)
Expand Down
15 changes: 7 additions & 8 deletions tests/assets/recipes/baked/daylight-factor.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ dependencies:
tag: 1.2.3
source: https://example.com/test-repo
flow:
- name: 5bb562a0b8fbbbebbf7851a7a9c0ddeb771455ff5a36d03d0998eba707bc80d5/main
- name: 3ea26da1d38c0f4a5efae830596f684257a91fd34316d1a0dce3ec2acbaad736/main
inputs:
parameters:
- annotations: {}
Expand Down Expand Up @@ -103,12 +103,12 @@ flow:
name: split-grid
variable: grid-list
value: null
sub_folder: null
sub_folder: output/raw-results
outputs:
parameters: []
artifacts:
- name: result-file
path: grid.res
path: '{{item.name}}.res'
- name: generate-sky
template: 3975f9c3104ac18e6559b4c92f4c4d23e1eeb1070a1298df4343e1c7e7307cff/10000-lux-sky
arguments:
Expand All @@ -129,9 +129,8 @@ flow:
artifacts:
- name: raw-results-folder
from:
type: tasks
name: daylight-factor-simulation
variable: result-file
type: folder
path: output/raw-results
subpath: null
dependencies:
- daylight-factor-simulation
Expand All @@ -142,7 +141,7 @@ flow:
- name: daylight-factor-average
artifacts:
- name: post-process-folder
path: output/post-process
path: result
- name: split-grid
template: 3975f9c3104ac18e6559b4c92f4c4d23e1eeb1070a1298df4343e1c7e7307cff/split-grid
arguments:
Expand Down Expand Up @@ -180,7 +179,7 @@ flow:
type: tasks
name: post-process
variable: post-process-folder
digest: 5bb562a0b8fbbbebbf7851a7a9c0ddeb771455ff5a36d03d0998eba707bc80d5
digest: 3ea26da1d38c0f4a5efae830596f684257a91fd34316d1a0dce3ec2acbaad736
templates:
- name: 3975f9c3104ac18e6559b4c92f4c4d23e1eeb1070a1298df4343e1c7e7307cff/10000-lux-sky
description: Generates a 10000 lux sky
Expand Down
30 changes: 13 additions & 17 deletions tests/assets/recipes/baked/parametric-daylight-factor.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ dependencies:
tag: 0.0.1
source: https://example.com/test-repo
flow:
- name: 670b9ea331cb4aa1db66d87bfb3816e45d6e986c540d527cf307d3b26cabdfa3/main
- name: aff667ff2b510fd7453bde01286c632912870e0c597fd414a002deed15ee557f/main
inputs:
parameters:
- annotations: {}
Expand All @@ -38,7 +38,7 @@ flow:
fail_fast: true
tasks:
- name: daylight-factor
template: 5bb562a0b8fbbbebbf7851a7a9c0ddeb771455ff5a36d03d0998eba707bc80d5/main
template: 3ea26da1d38c0f4a5efae830596f684257a91fd34316d1a0dce3ec2acbaad736/main
arguments:
parameters: []
artifacts:
Expand All @@ -58,22 +58,19 @@ flow:
type: inputs
variable: model-names
value: null
sub_folder: null
sub_folder: daylight-factor/{{item}}
outputs:
parameters:
- name: average
artifacts:
- name: data
path: daylight-factor.res
artifacts: []
outputs:
parameters: []
artifacts:
- name: daylight-factor-grids
from:
type: tasks
name: daylight-factor
variable: data
digest: 670b9ea331cb4aa1db66d87bfb3816e45d6e986c540d527cf307d3b26cabdfa3
type: folder
path: daylight-factor
digest: aff667ff2b510fd7453bde01286c632912870e0c597fd414a002deed15ee557f
templates:
- name: 3975f9c3104ac18e6559b4c92f4c4d23e1eeb1070a1298df4343e1c7e7307cff/10000-lux-sky
description: Generates a 10000 lux sky
Expand Down Expand Up @@ -201,7 +198,7 @@ templates:
registry: null
workdir: /opt/run/
local: null
- name: 5bb562a0b8fbbbebbf7851a7a9c0ddeb771455ff5a36d03d0998eba707bc80d5/main
- name: 3ea26da1d38c0f4a5efae830596f684257a91fd34316d1a0dce3ec2acbaad736/main
inputs:
parameters:
- annotations: {}
Expand Down Expand Up @@ -283,12 +280,12 @@ templates:
name: split-grid
variable: grid-list
value: null
sub_folder: null
sub_folder: output/raw-results
outputs:
parameters: []
artifacts:
- name: result-file
path: grid.res
path: '{{item.name}}.res'
- name: generate-sky
template: 3975f9c3104ac18e6559b4c92f4c4d23e1eeb1070a1298df4343e1c7e7307cff/10000-lux-sky
arguments:
Expand All @@ -309,9 +306,8 @@ templates:
artifacts:
- name: raw-results-folder
from:
type: tasks
name: daylight-factor-simulation
variable: result-file
type: folder
path: output/raw-results
subpath: null
dependencies:
- daylight-factor-simulation
Expand All @@ -322,7 +318,7 @@ templates:
- name: daylight-factor-average
artifacts:
- name: post-process-folder
path: output/post-process
path: result
- name: split-grid
template: 3975f9c3104ac18e6559b4c92f4c4d23e1eeb1070a1298df4343e1c7e7307cff/split-grid
arguments:
Expand Down
13 changes: 5 additions & 8 deletions tests/assets/recipes/folders/daylight-factor/flow/main.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ tasks:
type: tasks
name: split-grid
variable: grid-list
sub_folder:
- item.name
sub_folder: output/raw-results
arguments:
parameters:
- name: radiance-parameters
Expand All @@ -101,9 +100,8 @@ tasks:
variable: scene-file
outputs:
artifacts:
# This file is persisted in a loop sub-folder so the name won't clash
- name: result-file
path: grid.res
path: '{{item.name}}.res'

- name: post-process
template: honeybee-radiance/post-process
Expand All @@ -113,13 +111,12 @@ tasks:
artifacts:
- name: raw-results-folder
from:
type: tasks
name: daylight-factor-simulation
variable: result-file
type: folder
path: output/raw-results
outputs:
artifacts:
- name: post-process-folder
path: output/post-process
path: result
parameters:
- name: daylight-factor-average

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ tasks:
from:
type: inputs
variable: model-names
sub_folder: 'daylight-factor/{{item}}'
arguments:
artifacts:
- name: model
Expand All @@ -32,18 +33,13 @@ tasks:
outputs:
parameters:
- name: average
artifacts:
# This file is persisted in a loop sub-folder so the name won't clash
- name: data
path: daylight-factor.res

outputs:
artifacts:
- name: daylight-factor-grids
from:
type: tasks
name: daylight-factor
variable: data

type: folder
path: daylight-factor



1 change: 1 addition & 0 deletions tests/assets/recipes/invalid/ref-loop-artifact.error
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Cannot refer to outputs from a looped task.You must perform your own aggregation and then refer to a hard coded folder path.
30 changes: 30 additions & 0 deletions tests/assets/recipes/invalid/ref-loop-artifact.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
metadata:
name: minimal-recipe
tag: 0.2.1

dependencies:
- type: operator
name: honeybee-radiance
tag: 1.2.3
source: https://example.com/test-repo

flow:
- name: main
tasks:
- name: minimal-task
template: honeybee-radiance/10000-lux-sky
loop:
value:
- 1
- 2
outputs:
artifacts:
- name: sky
path: '{{item}}.sky'
outputs:
artifacts:
- name: skies
from:
type: tasks
name: minimal-task
variable: sky
34 changes: 18 additions & 16 deletions tests/assets/recipes/valid/daylight-factor.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,29 +25,33 @@ flow:
inputs:
parameters:
- name: sensor-grid-count
annotations: {}
description: The maximum number of grid points per parallel execution
default: 100
- name: radiance-parameters
annotations: {}
description: The radiance parameters for ray tracing
default: -I -ab 2 -h

artifacts:
- name: model
annotations: {}
description: A Honeybee model with radiance properties
required: true
- name: input-grid
annotations: {}
description: A grid file
required: true

tasks:

- name: generate-sky
template: honeybee-radiance/10000-lux-sky
outputs:
artifacts:
- name: sky
path: asset/sky/10000_lux.sky

- name: split-grid
template: honeybee-radiance/split-grid
arguments:
Expand All @@ -67,7 +71,7 @@ flow:
artifacts:
- name: output-grids-folder
path: output/temp

- name: create-octree
template: honeybee-radiance/create-octree
dependencies:
Expand Down Expand Up @@ -98,8 +102,7 @@ flow:
type: tasks
name: split-grid
variable: grid-list
sub_folder:
- item.name
sub_folder: output/raw-results
arguments:
parameters:
- name: radiance-parameters
Expand All @@ -120,10 +123,9 @@ flow:
variable: scene-file
outputs:
artifacts:
# This file is persisted in a loop sub-folder so the name won't clash
- name: result-file
path: grid.res

path: '{{item.name}}.res'
- name: post-process
template: honeybee-radiance/post-process
dependencies:
Expand All @@ -132,17 +134,16 @@ flow:
artifacts:
- name: raw-results-folder
from:
type: tasks
name: daylight-factor-simulation
variable: result-file
type: folder
path: output/raw-results
outputs:
artifacts:
- name: post-process-folder
path: output/post-process
path: result
parameters:
- name: daylight-factor-average


outputs:
artifacts:
- name: data
Expand All @@ -155,4 +156,5 @@ flow:
from:
type: tasks
name: post-process
variable: daylight-factor-average
variable: daylight-factor-average

Binary file not shown.
2 changes: 1 addition & 1 deletion tests/recipe/baked_recipe_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ def test_from_folder(self, folder):
parsed_instance = BakedRecipe.from_folder(folder)

parsed_test_path = os.path.join(ASSET_FOLDER, 'baked', f'{parsed_instance.metadata.name}.yaml')

parsed_instance.to_yaml(parsed_test_path)
if os.path.exists(parsed_test_path):
compare_instance = BakedRecipe.from_file(parsed_test_path)

Expand Down

0 comments on commit 0e99e92

Please sign in to comment.