cylc
Logging of unremovable tasks
Currently (as of #7101) we log a warning when cylc remove is unable to remove a task.
WARNING - Task(s) not removable: 1/foo, 1/bar
This is done for clarity as users may try to remove unremovable tasks and get confused when nothing happens.
There might be cases where this could be considered undesirable, however:
The comment suggests it's not just group trigger where this warning can be erroneous.
I found one example:
cylc trigger task --flow=2 cylc remove FAMILY --flow=2(where
taskis a member ofFAMILY)
https://github.com/cylc/cylc-flow/pull/7101#discussion_r2581062880
Personally I'm not sure this is a problem.
[!NOTE] This warning is not emitted when group-triggering, don't worry about group trigger
Note
This warning is not emitted when group-triggering, don't worry about group trigger
(It is emitted during group trigger on master, but presumably you mean in the current implementation of #7101).
So ... this used to be a warning, because prior to extending remove beyond the task pool it meant "remove a task proxy from the n=0 window" which simply doesn't make sense for other (n>0) tasks.
I changed it to debug level during group trigger implementation (post extended remove) because now:
- we can remove any task with history, and attempting to remove history-less tasks is a just harmless no-op (with less implication of error than attempting to remove n>0 tasks in the old regime, IMO)
- triggering a group after task failures is a common operation and almost guaranteed to (unhelpfully) generate these warnings (from auto-removing group members that haven't run yet downstream of failed tasks).
The comment suggests it's not just group trigger where this warning can be erroneous:
# This often does not indicate an error - e.g. for group trigger.
On reflection, I don't think we need to log a warning here even for standalone (non-trigger) use of remove - attempting to remove a history-less task is a harmless no-op, as noted above, and it may be convenient to do that as part of a group operation now that task matching extends beyond n=0.
So I think we can close this after reverting back to debug level on #7101, and drop the new warn_unremovable flag there too.
So ... this used to be a warning, because prior to extending remove beyond the task pool it meant "remove a task proxy from the n=0 window" which simply doesn't make sense for other (n>0) tasks.
No, actually this warning was introduced when I extended remove beyond the task pool, in #6472. The warning is not emitted if cylc remove was able to remove flow history. Only if the remove is a total no-op does it emit a warning. And I think users will accidentally do no-op removes fairly frequently - the clarity is helpful IMO.
triggering a group after task failures is a common operation and almost guaranteed to (unhelpfully) generate these warnings
cylc trigger will not generate this warning on #7101 (or on master as it is currently at debug level).
No, actually this warning was introduced when I extended remove beyond the task pool, in https://github.com/cylc/cylc-flow/pull/6472.
Prior to the remove extension we definitely did emit a warning on trying to remove n>0 tasks - whatever the exact string was, same difference - it meant the task was not removable because it did not exist in the task pool.
Only if the remove is a total no-op does it emit a warning.
Agreed - now that "remove" means "remove history" it would be silly to say the task was unremovable if it had history to remove.
cylc trigger will not generate this warning on https://github.com/cylc/cylc-flow/pull/7101 (or on master as it is currently at debug level).
That sounds like you think you're disagreeing with me, but you're not:
RD This warning is not emitted when group-triggering, don't worry about group trigger
HO (It is emitted during group trigger on master, but presumably you mean in the current implementation of https://github.com/cylc/cylc-flow/pull/7101).
I meant the message is emitted on master, which it is, but I guess you meant specifically at warning level (OK my bad for interpreting that too loosely) and the second half of my sentence agrees that it is not emitted in the implementation of 7101.
Only if the remove is a total no-op does it emit a warning. And I think users will accidentally do no-op removes fairly frequently - the clarity is helpful IMO.
You didn't directly respond to my argument, which was this:
attempting to remove a history-less task is a harmless no-op ... and it may be convenient to do that as part of a group operation now that task matching extends beyond n=0.
To be a bit more explicit:
(1) "Group operation" means (e.g.) cylc remove FAM not just cylc trigger FAM, so even if you deliberately disable the warning for trigger operations (as you have done) users can still get the warning if they try to remove a group of tasks. And if they are likely to do that for a group containing failed tasks, that group will likely contain history-less tasks downstream of the failed ones - I don't see how the warning is helpful in that case because there is nothing that the user should have done differently. (It should still debug-logged though, to help us track events).
(2) The other way is cylc remove <future-task>. Remove erases history, and whether the target task has history or not the result is the same: no history (after the command) - so why does that warrant a warning, exactly? I don't think the reason can be "they tried to remove a future task which is a no-op, so maybe that means they got the task ID wrong and they really meant to target an active or past task" - our commands should respond to the arguments given, not try to second guess what the user meant.
Finally I'm not sure it's legit to have a remove warning that is disabled when the exact same operation happens via the trigger command. In principle you could still do it the long way ( cylc remove then cylc trigger) - why should the warnings be any different?
I get what you're saying about removing a family. But from a support perspective it makes a night and day difference if you're able to see what tasks were and were not removed when a user ran the command.
our commands should respond to the arguments given, not try to second guess what the user meant.
Isn't that what this warning is doing? It's stating that tasks that the user tried to remove were not removable. What it isn't doing is assuming the user knew what they were doing!
As a compromise we could demote this to INFO level?