Fixes #21118 - Verify if there are no any running tasks #114
Conversation
| failure_message(task_count), | ||
| :next_steps => | ||
| [Procedures::ForemanTasks::Resume.new, | ||
| Procedures::ForemanTasks::UiInvestigate.new('search_query' => 'state = paused')]) |
There was a problem hiding this comment.
The search query should be state = running, shouln't it?
| assert(task_count == 0, | ||
| failure_message(task_count), | ||
| :next_steps => | ||
| [Procedures::ForemanTasks::Resume.new, |
There was a problem hiding this comment.
The resume is not valid for running tasks. We could have a procedure, that polls and waits for the tasks to finish.
There was a problem hiding this comment.
In the running_tasks_count, I have mentioned filter state field should be either running or paused. That's why I mentioned this step.
Should I remove paused from state filter values?
There was a problem hiding this comment.
@iNecas ,
I have queries regarding this:
- Do we need to set any timeout for this procedure to exit? Or we really need to wait till all tasks to finish.
- What interval should we set before retrying status of tasks?
There was a problem hiding this comment.
We have the ForemanTasks::NotPaused check for the paused, so this should be for running only.
Having a timeout is a good idea. I would suggest something around 5 minutes for start.
For the retry interval, I would go for every 10 seconds, as it's not very expensive operation.
There was a problem hiding this comment.
@iNecas ,
Is it okay if I change order for NotRunning check as it is showing after NotPaused check?
I know that it is a rare situation where any running task which will get paused somehow in NotRunning check. But in that scenario it will proceed with upgrade.
I have pushed the changes but they are incomplete. I just saw your comments.
I will update PR.
kgaikwad
force-pushed
the
21118_detect_if_no_running_tasks
branch
2 times, most recently
from
5c3231f
to
6012b93
Compare
|
@iNecas , Updated the PR with |
| end | ||
|
|
||
| def run | ||
| with_spinner("wait till #{@state} tasks get finish") do |spinner| |
There was a problem hiding this comment.
till is quite informal, let's use until instead. I would also recommend using ing from, as we are describing, what's happening at the moment, and instead of get finish, it would sound better to me to waiting for 2 tasks to finish or waiting until tasks get finished.
I would recommend waiting for #{@state} tasks to finish
| failure_message(task_count), | ||
| :next_steps => | ||
| [Procedures::ForemanTasks::FetchTasksStatus.new(:state => 'running'), | ||
| Procedures::ForemanTasks::UiInvestigate.new('search_query' => 'state = running')]) |
There was a problem hiding this comment.
The search query should be state = running AND label !^(#{Features::ForemanTasks::EXCLUDE_ACTIONS_FOR_RUNNING_TASKS.join(" ")})
There was a problem hiding this comment.
The UiInvestigate procedure is using press ENTER after the paused tasks are resolved wording, which is not precise for this case. Using press ENTER after the tasks are resolved would be matching both running and paused tasks.
kgaikwad
force-pushed
the
21118_detect_if_no_running_tasks
branch
from
6012b93
to
0d54f38
Compare
|
@iNecas , Sorry for small mistakes. |
kgaikwad
force-pushed
the
21118_detect_if_no_running_tasks
branch
from
0d54f38
to
7b8d45b
Compare
|
Thanks @kgaikwad, tested and works well. Merging and will release a new version. Enjoy the holiday! |
|
Thank you !! 😊 |
|
0.0.10 is out now |
No description provided.