-
Notifications
You must be signed in to change notification settings - Fork 94
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Decide on the future of naked tasks. #3866
Comments
I think we need to change terminology. If we really want to allow naked tasks to only generate a warning by default (not my preference) then I'd suggest we add a |
I may be coming around to your point of view. To be fair it should be more important to avoid accidental "implicit tasks" caused by a simple typo, than to make quick "experiment with scheduling" flows super-easy to write. How about disallow them, but provide a command line one-off opt-in capability? |
The case for naked tasksThe majority of the complexity of a workflow lies in the As a result I would encourage users to start writing a workflow with the Naked tasks reduce the barrier to experimentation and rapid-prototyping making learning and workflow development easier, especially for new users as they feel their way around Cylc's scheduling concepts. The documentation is full of examples which use naked tasks to simplify examples cutting out needless complexity. Here are three illustrative examples chosen at random: Were we to forbid naked tasks or require use of an option or configuration all of these previously valid examples would fail despite being fundamentally fine. There are also functional uses for naked tasks, consider the common pattern: [runtime]
[[root]]
script = rose task-run If a task does not require explicit configuration what is the merit to enforcing it? Requiring users to manually list their tasks could prove a headache in some cases. What if tasks are programatically generated? The case for protecting against naked tasksUnintentional naked tasks are typos which change the name of a task in a graph section to something which isn't defined in the runtime section. This does not cover all potential typos but does cover all typos that we can realistically capture. This is an especially important integrity check in production environments where we would expect each task to be individually defined and configured. |
@oliver-sanders has nicely described exactly how I've always viewed and used naked tasks. So I tend to agree that we shouldn't prevent that if possible. The trouble with opt-in disallow (via CLI option or config setting) is that production users may not know about it, or may forget to use it. The trouble with opt-in allow via config setting is that production users may forget to remove it after moving to production mode. So how about (as I suggested above) we support opt-in use of naked tasks via CLI option (and something equivalent for the GUI):
This retains safety automatically for production users - who are unlikely to use such an option accidentally! - and it's a very minor imposition on those experimenting with the scheduling section. We can even highlight the existence of the option in the error message when naked tasks are detected, so that casual users don't have to remember it... |
Kinda prefer the idea of just warning for naked tasks (after all they are not errors) and providing an opt-in "turn warnings into errors" option for production environments. Opt-out might be ok-ish if we document it properly, note that commands like |
That's OK with me, but I can see @dpmatthews point given that opt-in safety checks are easy to miss or forget about. Let's see what he thinks... |
I think opt-in safety checks are easy to miss or forget about. |
We need to agree a way forward on this. [scheduling]
allow implicit = True We'd encourage users to set this whilst doing experimentation/ prototyping in the |
Agreed this may be the best option 😠, though |
Agreed, good solution 🎉 |
Do we want to keep the |
We would really like to get rid of |
Looks like cylc-flow/cylc/flow/scripts/validate.py Lines 104 to 120 in 66c1119
|
Yeah, verbose will do. I'm not sure why we had that check only for |
Looking at git blame, #2921 changed it from an error to a warning, so that's probably why it's only for |
I think always log the warning and get rid of the verbose junk and maybe get rid of the special verbose logging, not sure why that prints out the same message in a different format. At the time of #2921 We are hoping that |
Should they stay or should they go?
cylc/cylc-admin#111 (review)
[edit]
"Naked" tasks are tasks without an explicit runtime definition i.e in this example
bar
is a "naked" task:Naked tasks should cause configuration errors by default with this behaviour overridden by a new config:
We have referred to these as dummy or naked-dummy tasks in the past, I think we will go with "implicit" from here on?
[/edit]
The text was updated successfully, but these errors were encountered: