Skip to content
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

chore/add concurrency support for deployments #306

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jimid27
Copy link
Contributor

@jimid27 jimid27 commented Nov 21, 2024

This object in the API is pretty simple, with only one nested attribute.
It's not so flexible that we need to default to a basic JSON object.
@mitchnielsen
Copy link
Contributor

mitchnielsen commented Nov 22, 2024

🤔 Looking into concurrency_limit more. Looks like it is deprecated in favor of concurrency_limit_id based on this code. That change was added in PrefectHQ/prefect#15426, which has a lot more context on the change. I need to read up on that to understand how we need to implement the ability in the Terraform provider.

The relevant docs (https://docs.prefect.io/v3/deploy/index#concurrency-limiting) still instruct you to set concurrency_limit directly, and the API docs don't say it's deprecated. And from the UI it looks like the deprecated option is still in use:

image

I'll read up more and reach out to the backend folks if anything needs clarification.

PrefectHQ/prefect#15426 (comment):

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.

This explains why the tests here were failing for me, saying we set 1 but the result came back as 0.

Overall: looks like this field is going away next month (Dec 2024), so we probably want to implement the newer approach.

Further reading:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants