queue: update kill ui messages #8798
Conversation
dvc/repo/experiments/queue/celery.py
Outdated
| @@ -316,7 +319,7 @@ def _try_to_kill_tasks( | |||
| self.proc.kill(queue_entry.stash_rev) | |||
| else: | |||
| self.proc.interrupt(queue_entry.stash_rev) | |||
| logger.debug(f"Task {rev} had been killed.") | |||
| ui.write(f"Task {rev} has been killed.") | |||
There was a problem hiding this comment.
Task or expriment?
(in VS Code we won't be able to use Task, for DVC it might make sense if command is dvc queue ...)
There was a problem hiding this comment.
I don't think either is necessary here, so I just dropped "Task."
dvc/repo/experiments/queue/celery.py
Outdated
| @@ -316,7 +319,7 @@ def _try_to_kill_tasks( | |||
| self.proc.kill(queue_entry.stash_rev) | |||
| else: | |||
| self.proc.interrupt(queue_entry.stash_rev) | |||
| logger.debug(f"Task {rev} had been killed.") | |||
| ui.write(f"Task {rev} has been killed.") | |||
There was a problem hiding this comment.
Let's move ui messages to respective commands. It's hard to change existing ones, but newer commands should try to put UI related outputs in commands as much as possible, and internals should be designed in such a way.
There was a problem hiding this comment.
If we show this to the users maybe we should also show the message in line 340
There was a problem hiding this comment.
If we show this to the users maybe we should also show the message in line 340
Not sure I follow @karajan1001. You mean Task id {task_id} rev {remained_entries[entry]} marked as failure? I'm not sure it's needed.
@skshetry Do you have a suggestion for how to do it here? I'd like for any valid tasks to get killed and show a success message for each as they are killed, and finally return an error if there were any invalid tasks.
There was a problem hiding this comment.
ping @karajan1001 @skshetry
|
There are merge conflicts, @dberenbaum. Could you please resolve them? |
Codecov ReportBase: 93.71% // Head: 93.72% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #8798 +/- ##
=======================================
Coverage 93.71% 93.72%
=======================================
Files 453 453
Lines 36146 36154 +8
Branches 5239 5240 +1
=======================================
+ Hits 33875 33884 +9
Misses 1761 1761
+ Partials 510 509 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Fix #8797