Execute QueueManager items in order #3571
Conversation
|
|
| self.async_register_repository( | ||
| repository_full_name=repo, | ||
| category=category, | ||
| default=True, | ||
| ) | ||
| ) | ||
| self.queue.add(tasks) |
There was a problem hiding this comment.
Where are the tasks in the queue cleaned up if there are remaining tasks on shutdown?
We should probably not add coroutines directly but either coroutine functions or asyncio.Tasks so we avoid running into "coroutine was never awaited".
There was a problem hiding this comment.
This is a valid point, but I think that should be changed separately as it doesn't really have anything to do with the changes in this PR,
|
The reason for this PR isn't really clear in the PR description. |
emontnemery
force-pushed
the
queue_manager_execution_order
branch
from
a05c858 to
068d0c9
Compare
emontnemery
force-pushed
the
queue_manager_execution_order
branch
from
10f74d2 to
3105551
Compare
|
|
||
| @property | ||
| def has_pending_tasks(self) -> bool: | ||
| """Return a count of pending tasks in the queue.""" | ||
| """Return if there are pending tasks in the queue.""" | ||
| return self.pending_tasks != 0 |
There was a problem hiding this comment.
| return self.pending_tasks != 0 | |
| return len(self.queue) != 0 |
As we remove things from queue this should be a simple count instead of pending_tasks since that one now does a sum of all childs
| for task in queue_item: | ||
| local_queue.append(task) | ||
|
|
||
| for task in local_queue: |
There was a problem hiding this comment.
Looking now, this looks like a bug?
These should be removed after the items have been executed right?
So the properties exposed in this class would be correct.
There was a problem hiding this comment.
I think it's not obvious what is correct here. If removing the items early, we consider the tasks no longer in the queue when execution starts. If removing the items late, we consider the tasks no longer in the queue when execution is finished.
|
After some discussion on discord, I'm not sure we should merge this. |
|
Let's not move forward with this one, but keep a mental note about the known queue items cleanup for the exposed properties. |
Execute
QueueManageritems in order and change queue items to beIterable[Coroutine]where the iterable items are executed in parallel and thus not guaranteed to be executed in order.This is needed by #3572