Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions cwltool/checker.py
Original file line number Diff line number Diff line change
Expand Up @@ -592,3 +592,16 @@ def loop_checker(steps: Iterator[MutableMapping[str, Any]]) -> None:
)
if exceptions:
raise ValidationException("\n".join(exceptions))


def resreq_minmax_checker(requirement: CWLObjectType) -> None:
"""
Check if the minResource is lesser than the maxResource in resource requirements.

:raises ValidationException: If the minResource is greater than the maxResource.
"""
for a in ("cores", "ram", "tmpdir", "outdir"):
mn = requirement.get(a + "Min")
mx = requirement.get(a + "Max")
if isinstance(mn, (int, float)) and isinstance(mx, (int, float)) and mx < mn:
raise ValidationException(f"{a}Min cannot be greater than {a}Max.")
Comment on lines +603 to +607
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work in the case of a CWL Expression or Parameter Reference (type str).

Either
A. Skip strings here
or
B. move the call to this function to later, when a builder object is available to utilize builder.do_eval to get the actual values.

Copy link
Copy Markdown
Author

@Stellatsuu Stellatsuu Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @mr-c,

I removed the cast here so that string values are skipped. This part of the code is only executed when running cwltool --validate. The actual runtime validation is performed later in builder.resources = self.evalResources(builder, runtime_context), so Expressions should work here.

My question is: if we want to --validate a requirement that contains a CWL expression, we first need to evaluate the expression to get an int or float before running the validation. But since evaluating expressions requires a Builder, how could this be done during validation? Validation does not use builder right?

5 changes: 5 additions & 0 deletions cwltool/command_line_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
content_limit_respected_read_bytes,
substitute,
)
from .checker import resreq_minmax_checker
from .context import LoadingContext, RuntimeContext, getdefault
from .docker import DockerCommandLineJob, PodmanCommandLineJob
from .errors import UnsupportedRequirement, WorkflowException
Expand Down Expand Up @@ -410,6 +411,10 @@ def __init__(self, toolpath_object: CommentedMap, loadingContext: LoadingContext
else PathCheckingMode.STRICT
)

resource_req = self.get_requirement("ResourceRequirement")[0]
if getdefault(loadingContext.do_validate, True) and resource_req:
resreq_minmax_checker(resource_req)

def make_job_runner(self, runtimeContext: RuntimeContext) -> type[JobBase]:
"""Return the correct CommandLineJob class given the container settings."""
dockerReq, dockerRequired = self.get_requirement("DockerRequirement")
Expand Down
10 changes: 5 additions & 5 deletions cwltool/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -966,9 +966,7 @@ def inc(d: list[int]) -> None:
bindings_copy = copy.deepcopy(bindings)
del bindings[:]
bindings.extend(sorted(bindings_copy, key=key))

if self.tool["class"] != "Workflow":
builder.resources = self.evalResources(builder, runtime_context)
builder.resources = self.evalResources(builder, runtime_context)
return builder

def evalResources(
Expand Down Expand Up @@ -1027,9 +1025,11 @@ def evalResources(
elif mx is None:
mx = mn

if mn is not None:
if mn is not None and mx is not None:
if mx < mn:
raise ValidationException(f"{a}Min cannot be greater than {a}Max.")
request[a + "Min"] = mn
request[a + "Max"] = cast(Union[int, float], mx)
request[a + "Max"] = mx

request_evaluated = cast(dict[str, Union[int, float]], request)
if runtimeContext.select_resources is not None:
Expand Down
11 changes: 10 additions & 1 deletion cwltool/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@
from schema_salad.sourceline import SourceLine, indent

from . import command_line_tool, context, procgenerator
from .checker import circular_dependency_checker, loop_checker, static_checker
from .checker import (
circular_dependency_checker,
loop_checker,
resreq_minmax_checker,
static_checker,
)
from .context import LoadingContext, RuntimeContext, getdefault
from .cwlprov.provenance_profile import ProvenanceProfile
from .cwlprov.writablebagfile import create_job
Expand Down Expand Up @@ -133,6 +138,10 @@ def __init__(
circular_dependency_checker(step_inputs)
loop_checker(step.tool for step in self.steps)

resource_req = self.get_requirement("ResourceRequirement")[0]
if resource_req:
resreq_minmax_checker(resource_req)

def make_workflow_step(
self,
toolpath_object: CommentedMap,
Expand Down
15 changes: 15 additions & 0 deletions tests/test_examples.py
Original file line number Diff line number Diff line change
Expand Up @@ -1940,3 +1940,18 @@ def test_make_template() -> None:
]
)
assert exit_code == 0, stderr


@pytest.mark.parametrize(
"file", ["tests/wf/bad_resreq_mnmx_clt.cwl", "tests/wf/bad_resreq_mnmx_wf.cwl"]
)
Comment thread
mr-c marked this conversation as resolved.
def test_invalid_resource_requirements(file: str) -> None:
"""Ensure an error with an invalid resource requirement."""
exit_code, stdout, stderr = get_main_output(
[
"--disable-validate",
get_data(file),
]
)
assert exit_code == 1
assert "cannot be greater than" in stderr
17 changes: 17 additions & 0 deletions tests/test_validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import logging
import re

import pytest

from .util import get_data, get_main_output


Expand Down Expand Up @@ -125,3 +127,18 @@ def test_validate_custom_logger() -> None:
assert "tests/CometAdapter.cwl#out' previously defined" not in stdout
assert "tests/CometAdapter.cwl#out' previously defined" not in stderr
assert "tests/CometAdapter.cwl#out' previously defined" in custom_log_text


@pytest.mark.parametrize(
"file", ["tests/wf/bad_resreq_mnmx_clt.cwl", "tests/wf/bad_resreq_mnmx_wf.cwl"]
)
def test_validate_with_invalid_requirements(file: str) -> None:
"""Ensure that --validate returns an error with an invalid resource requirement."""
exit_code, stdout, stderr = get_main_output(
[
"--validate",
get_data(file),
]
)
assert exit_code == 1
assert "cannot be greater than" in stdout
14 changes: 14 additions & 0 deletions tests/wf/bad_resreq_mnmx_clt.cwl
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
#!/usr/bin/env cwl-runner
class: CommandLineTool
cwlVersion: v1.2

requirements:
ResourceRequirement:
coresMin: 4
coresMax: 2

inputs: []
outputs: []

baseCommand: ["echo", "Hello World"]

33 changes: 33 additions & 0 deletions tests/wf/bad_resreq_mnmx_wf.cwl
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#!/usr/bin/env cwl-runner
class: Workflow
cwlVersion: v1.2

requirements:
ResourceRequirement:
ramMin: 128
ramMax: 64

inputs:
message: string?

outputs: []

steps:
hello_world:
requirements:
ResourceRequirement:
ramMin: 64
ramMax: 128
run:
class: CommandLineTool
baseCommand: echo
inputs:
message:
type: string?
default: "Hello World"
outputs: []
arguments:
- $(inputs.message)
in:
message: message
out: []