-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Fix stale deployment statuses #14095
Conversation
@@ -1845,28 +1845,52 @@ async def test_updates_last_polled_on_a_multiple_work_queues( | |||
assert work_queue.last_polled is None | |||
|
|||
async def test_updates_last_polled_on_a_full_work_pool( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes to this test adds coverage that all requested work queues' last polled field gets updated regardless of their status which covers my changes to the polled_work_queue_ids
up in the app code.
assert work_queue.last_polled > now | ||
|
||
async def test_updates_work_queue_status_on_a_full_work_pool( | ||
self, client, session, work_queues, work_pools | ||
async def test_updates_statuses_on_a_full_work_pool( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test now covers that both work queue statuses and deployment statuses are updated here.
There's one more pre-existing test after this one that checks a single deployment is updated correctly but this covers that multiple relevant deployments are updated regardless of work queue statuses
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks sharp to me!
) | ||
|
||
background_tasks.add_task( | ||
mark_deployments_ready, | ||
work_queue_ids=ready_work_queue_ids, | ||
work_queue_ids=[wq.id for wq in work_queues], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
Similar (but different) to #14082 - this PR fixes a separate issue where Deployment statuses aren't being updated correctly. In particular, a work pool + queue with a running worker does not update a newly created deployment to READY.
Closes #13869
See #14097 for 2.x port.
Checklist
maintenance
,fix
,feature
,enhancement
,docs
.<link to issue>
"mint.json
.