cylc / cylc-admin

Project planning for the Cylc Workflow Engine.
https://cylc.github.io/cylc-admin/
GNU General Public License v3.0
5 stars 13 forks source link

Modify the optional outputs extension proposal #181

Closed dpmatthews closed 11 months ago

dpmatthews commented 11 months ago

A replacement for the optional outputs extension agreed in #173.

hjoliver commented 11 months ago

Oliver, above:

This proposal handles the expire problem by subtly redefining the meaning of "required" to cover execution only, i.e. if this task runs it must succeed, but it might not run; rather than handing both at the same time, i.e. this task might not succeed because it might not run.

Not that it really matters now, but this is what I argued for in my original proposal.

Me, above:

For expiry, it's like my original proposal but with the additional safety net ...

Contrary to what it says (but not added in this PR) under the section "Preference To The Expire Proposal" my original proposal didn't really "recommend converting expiry from an output to a task attribute".

The main points of it were:

On the last point, it said:

Required outputs are always conditional on the owner task actually running, and an expired task does not run at all. So :succeed (and other outputs) can still be required even if expiration is optional. Required success really means, if the task runs, its success is required.

Unfortunately I neglected to flag :submit-fail as being similar to :expire in this respect, doh.

And also:

NOTE this is moot if [IF] we decide that :expire should not be considered a task output - see below.

i.e., while I argued at the bottom that expired should really be an attribute not a state/output, that wasn't supposed to be a fundamental part of the proposal. (It was an option, albeit one I quite liked).

Anyway, I'm happier now, because expiry is more like originally intended AND safer!

hjoliver commented 11 months ago

Actually, before we merge this:

Forced expiry is not explicitly addressed here - i.e., manually telling the scheduler not to bother running a task.

To protect against early halt on overzealous use of forced expiry, I presume we should handle it like this:

OR can we say the user is always right when it comes to manual interventions, so do not make an incomplete task on manual task expire either.

dpmatthews commented 11 months ago

Forced expiry is not explicitly addressed here - i.e., manually telling the scheduler not to bother running a task.

Expiry is expiry - it makes no difference whether it is set via cylc set. So, the normal condition applies: a task in incomplete If the task expired and the:expire output was not optional.

If you want to not run a task, don't use expiry unless you've configured the workflow to handle it. You can use cylc set or skip mode or possibly cylc remove depending on what you're trying to achieve.

hjoliver commented 11 months ago

In that case, merging 🎉