Skip to content

Conversation

@Stellatsuu
Copy link

Related to : #2163

  • Added resource requirement minmax validation
  • Added validation when running --validate
  • Added related tests

Copy link
Member

@mr-c mr-c left a comment

Choose a reason for hiding this comment

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

Thank you @Stellatsuu !

@codecov
Copy link

codecov bot commented Nov 17, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.92%. Comparing base (0cbbd92) to head (4937dbc).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2179      +/-   ##
==========================================
+ Coverage   84.85%   84.92%   +0.07%     
==========================================
  Files          46       46              
  Lines        8509     8523      +14     
  Branches     1984     1988       +4     
==========================================
+ Hits         7220     7238      +18     
- Misses        812      814       +2     
+ Partials      477      471       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Stellatsuu
Copy link
Author

Stellatsuu commented Nov 19, 2025

cwltool --disable-validate tests/wf/bad_resreq_mnmx_wf.cwl command works while it shouldn't, since the workflow resource requirement is wrong:

#!/usr/bin/env cwl-runner
class: Workflow
cwlVersion: v1.2

requirements: <-----
  ResourceRequirement:
      ramMin: 128
      ramMax: 64

inputs: []
outputs: []

steps:
  hello_world:
    requirements:
      ResourceRequirement:
        ramMin: 64
        ramMax: 128
    run:
        class: CommandLineTool
        baseCommand: [ "echo", "Hello World" ]
        inputs: [ ]
        outputs: [ ]
    out: [ ]
    in: [ ]

I looked at the code, and it seems that the main reason is that, evalResources is only called when the cwl object is not a Workflow:

def Process(...):
   def _init_job(...):
        ...
        if self.tool["class"] != "Workflow":
            builder.resources = self.evalResources(builder, runtime_context)
        return builder
   def evalResources(...):
        -validation is here-

How should I manage to check if the resource requirements of a Workflow are correct when calling --disable-validate?

@Stellatsuu Stellatsuu requested a review from mr-c December 8, 2025 08:40
@Stellatsuu
Copy link
Author

Hello @mr-c,
I hope you're doing well.
Could you, please, take a quick look at this pull request? Especially this part: #2179 (comment) where I'm not sure of what to do to make it work as intended

Thank you in advance

@mr-c
Copy link
Member

mr-c commented Jan 6, 2026

Sorry for the delay, @Stellatsuu , I missed your original question; so thank you for the reminder

5f455a4 is where that variation was added, as part of implementing --parallel

Comment on lines 603 to 607
for a in ("cores", "ram", "tmpdir", "outdir"):
mn = cast(Union[int, float, None], requirement.get(a + "Min"))
mx = cast(Union[int, float, None], requirement.get(a + "Max"))
if mn is not None and mx is not None and mx < mn:
raise ValidationException(f"{a}Min cannot be greater than {a}Max.")
Copy link
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
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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants