-
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
Improve deployment concurrency GCL management #15426
Improve deployment concurrency GCL management #15426
Conversation
…ployment.concurrency_limit.
…or backwards-compatibility - rename expanded limit in the response.
This reverts commit 284d881.
CodSpeed Performance ReportMerging #15426 will not alter performanceComparing Summary
|
@@ -293,7 +293,7 @@ async def test_project_deploy(self, project_dir, prefect_client): | |||
assert deployment.work_pool_name == "test-pool" | |||
assert deployment.version == "1.0.0" | |||
assert deployment.tags == ["foo-bar"] | |||
assert deployment.concurrency_limit == 42 |
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.
Note that you can still access deployment.concurrency_limit
but it's now deprecated so it emits a warning on access and also it will be None
.
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.
I have a couple of questions about if/when we'll be able to remove fields deprecated in this PR, but otherwise LGTM!
Closes #15375
This PR includes several changes to support shifting the
int
-basedDeployment.concurrency_limit
over to a foreign key relationship betweenDeployment
andConcurrencyLimitV2
in order to centralize the source of truth for a deployment's concurrency limit value.More specifically, this PR:
deployment.concurrency_limit_id
column that is a nullable FK toconcurrency_limit_v2.id
. The migration will try to backfill pre-existing datadeployment.concurrency_limit
int valueslimit
integer in addition to other fields like the underlying GCL's nameNone
for client-side compatibility.DeploymentConcurrencyLimitFilter
for filtering deployments via the/filter
API. This still exists so as to not 422 on clients using this filter, but the filter will have no effect.Checklist
<link to issue>
"mint.json
.