Skip to content

Commit 1ca3962

Browse files
committed
dev: added nested_wf case and created additional functions to reduce cognitive complexity
1 parent efed860 commit 1ca3962

File tree

6 files changed

+150
-25
lines changed

6 files changed

+150
-25
lines changed

src/dirac_cwl_proto/execution_hooks/requirement_validator.py

Lines changed: 51 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,6 @@ def get_requirement(self, cwl_object: Workflow | CommandLineTool | WorkflowStep)
2525
return requirement
2626
return None
2727

28-
# TODO : get all nested workflows of a workflow,
29-
# for each nested workflow, validate its requirements
30-
def get_nested_workflows(self):
31-
pass
32-
3328
@abstractmethod
3429
def validate_requirement(
3530
self, requirement, context: str = "", global_requirement=None
@@ -55,54 +50,84 @@ def validate_production_requirement(self, requirement, is_global: bool = False):
5550
"""
5651
pass
5752

58-
def validate_requirements(self, production: bool = False) -> None:
53+
def validate_requirements(self, cwl_object=None, production: bool = False) -> None:
5954
"""
60-
Validate all requirements in Workflow, WorkflowStep and WorkflowStep.run.
55+
Validate all requirements in a Workflow.
6156
57+
:param cwl_object: The cwl_object to validate the requirements for.
58+
If None, use the cwl_object from the class. This is used for nested workflows validation.
6259
:param production: True if the requirements are for a production workflow, False otherwise.
63-
Maybe we could add a transformation parameter later.
60+
-- Maybe we could add a transformation parameter later.
6461
"""
65-
global_requirement = self.get_requirement(self.cwl_object)
62+
cwl_object = cwl_object if cwl_object else self.cwl_object
63+
global_requirement = self.get_requirement(cwl_object)
6664

6765
# Validate Workflow/CommandLineTool global requirements.
6866
if global_requirement:
67+
# Production-workflow-specific validation.
6968
if production:
7069
self.validate_production_requirement(global_requirement, is_global=True)
7170
self.validate_requirement(global_requirement, context="global requirements")
7271

7372
# Validate WorkflowStep requirements, if any.
74-
if self.cwl_object.class_ != "CommandLineTool" and self.cwl_object.steps:
75-
self.validate_steps_requirements(global_requirement, production)
73+
if cwl_object.class_ != "CommandLineTool" and cwl_object.steps:
74+
self.validate_steps_requirements(
75+
cwl_object.steps, global_requirement, production
76+
)
7677

77-
def validate_steps_requirements(self, global_requirement, production: bool = False):
78+
def validate_steps_requirements(
79+
self, steps, global_requirement, production: bool = False
80+
):
7881
"""
79-
Validate steps requirements in WorkflowStep and WorkflowStep.run.
82+
Validate steps requirements in WorkflowStep.
8083
84+
:param steps: The WorkflowStep to validate the requirements for.
8185
:param global_requirement: The global Workflow requirement, if any.
8286
:param production: True if the requirements are for a production workflow, False otherwise.
87+
-- Maybe we could add a transformation parameter later.
8388
"""
84-
# Validate WorkflowStep requirements, if any.
85-
for step in self.cwl_object.steps:
89+
for step in steps:
8690
step_requirement = self.get_requirement(step)
91+
# Validate WorkflowStep requirements, if any.
8792
if step_requirement:
93+
# Production-workflow-specific validation.
8894
if production:
8995
self.validate_production_requirement(step_requirement)
9096
self.validate_requirement(
9197
step_requirement,
9298
context="step requirements",
9399
global_requirement=global_requirement,
94100
)
101+
95102
# Validate WorkflowStep.run, if any.
96103
if step.run:
97-
step_run_requirement = self.get_requirement(step.run)
98-
if step_run_requirement:
99-
if production:
100-
self.validate_production_requirement(step_run_requirement)
101-
self.validate_requirement(
102-
step_run_requirement,
103-
context="step run requirements",
104-
global_requirement=global_requirement,
105-
)
104+
self.validate_run_requirements(step.run, global_requirement, production)
105+
106+
def validate_run_requirements(
107+
self, run, global_requirement, production: bool = False
108+
):
109+
"""
110+
Validate WorkflowStep.run requirements.
111+
112+
:param run: The WorkflowStep.run to validate the requirements for.
113+
:param global_requirement: The global Workflow requirement, if any.
114+
:param production: True if the requirements are for a production workflow, False otherwise.
115+
-- Maybe we could add a transformation parameter later.
116+
"""
117+
if run.class_ == "Workflow":
118+
# Validate nested Workflow requirements, if any.
119+
self.validate_requirements(cwl_object=run)
120+
121+
step_run_requirement = self.get_requirement(run)
122+
if step_run_requirement:
123+
# Production-workflow-specific validation.
124+
if production:
125+
self.validate_production_requirement(step_run_requirement)
126+
self.validate_requirement(
127+
step_run_requirement,
128+
context="step run requirements",
129+
global_requirement=global_requirement,
130+
)
106131

107132

108133
class RequirementError(Exception):
@@ -148,7 +173,8 @@ def validate_conflict(min_value, global_max_value, resource, context):
148173
def validate_production_requirement(self, requirement, is_global: bool = False):
149174
"""
150175
Raise an error if there's a global ResourceRequirement in a production workflow.
151-
Otherwise, add some logic for WorkflowSteps, CommandLineTools and WorkflowStep.run in production workflows.
176+
Otherwise, add some logic for WorkflowSteps, CommandLineTools
177+
and WorkflowStep.run, etc. in production workflows.
152178
153179
:param requirement: The current requirement to validate.
154180
:param is_global: True if there's a global ResourceRequirement, False otherwise.

test/test_workflows.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,16 @@ def test_run_job_success(cli_runner, cleanup, cwl_file, inputs):
279279
[],
280280
"ResourceRequirementisinvalid",
281281
),
282+
(
283+
"test/workflows/resource_requirements/resource_conflicts/nested_wf/external_conflict_nested_wf.cwl",
284+
[],
285+
"ResourceRequirementisinvalid",
286+
),
287+
(
288+
"test/workflows/resource_requirements/resource_conflicts/nested_wf/internal_conflict_nested_wf.cwl",
289+
[],
290+
"ResourceRequirementisinvalid",
291+
),
282292
],
283293
)
284294
def test_run_job_validation_failure(
@@ -699,6 +709,11 @@ def test_run_simple_production_success(cli_runner, cleanup, cwl_file, metadata):
699709
"test/workflows/mandelbrot/type_dependencies/production/malformed-nonexisting-type_metadata-mandelbrot_complete.yaml",
700710
"Unknownexecutionhooksplugin:'MandelBrotDoesNotExist'",
701711
),
712+
(
713+
"test/workflows/resource_requirements/bad_merge_production.cwl",
714+
"test/workflows/merge/type_dependencies/production/metadata-merge_complete.yaml",
715+
"ResourceRequirementisinvalid",
716+
),
702717
],
703718
)
704719
def test_run_production_validation_failure(
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
cwlVersion: v1.2
2+
class: Workflow
3+
label: "Higher NestedWorkflow resourceMin than resourceMax"
4+
doc: The NestedWorkflow ResourceRequirement has higher resourceMin than global resourceMax = failure, both NestedWorflows
5+
here are wrong but only the first one will trigger the exception.
6+
7+
inputs: []
8+
outputs: []
9+
10+
requirements:
11+
ResourceRequirement:
12+
ramMax: 512
13+
coresMax: 4
14+
15+
steps:
16+
too_high_ram:
17+
run: ./nested_wf_high_ram.cwl
18+
in: []
19+
out: []
20+
too_high_cores:
21+
run: ./nested_wf_high_cores.cwl
22+
in: []
23+
out: []
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
cwlVersion: v1.2
2+
class: Workflow
3+
label: "Higher NestedWorkflow resourceMin than resourceMax"
4+
doc: The NestedWorkflow ResourceRequirement has higher resourceMin than resourceMax = failure, both NestedWorflows
5+
here are wrong but only the first one will trigger the exception.
6+
7+
inputs: []
8+
outputs: []
9+
10+
steps:
11+
too_high_ram:
12+
run: ../ram_conflict_wf_step.cwl
13+
in: [ ]
14+
out: [ ]
15+
too_high_cores:
16+
run: ../cores_conflict_wf_step.cwl
17+
in: []
18+
out: []
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
cwlVersion: v1.2
2+
class: Workflow
3+
label: "Higher NestedWorkflow resourceMin than resourceMax"
4+
doc: This NestedWorkflow ResourceRequirement has higher coresMin than global coresMax = failure
5+
6+
inputs: []
7+
outputs: []
8+
9+
requirements:
10+
ResourceRequirement:
11+
coresMin: 10
12+
13+
steps:
14+
step:
15+
run:
16+
class: CommandLineTool
17+
baseCommand: ["echo", "Hello World"]
18+
inputs: []
19+
outputs: []
20+
in: []
21+
out: []
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
cwlVersion: v1.2
2+
class: Workflow
3+
label: "Higher NestedWorkflow resourceMin than resourceMax"
4+
doc: This NestedWorkflow ResourceRequirement has higher ramMin than global ramMax = failure
5+
6+
7+
inputs: []
8+
outputs: []
9+
10+
requirements:
11+
ResourceRequirement:
12+
ramMin: 1028
13+
14+
steps:
15+
step:
16+
run:
17+
class: CommandLineTool
18+
baseCommand: ["echo", "Hello World"]
19+
inputs: []
20+
outputs: []
21+
in: []
22+
out: []

0 commit comments

Comments
 (0)