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

Update deployment client API to take an Union type for concurrency_limit #15425

Merged

Conversation

jeanluciano
Copy link
Contributor

@jeanluciano jeanluciano commented Sep 18, 2024

Updates various client API related to creating and updating deployments to be able to take Union[int,ConcurrencyLimitConfig]. These changes can be summarized into:

  1. Updating methods like flow.deploy,flow.serve, and our prefect.yaml to take the union type.
  2. Updating RunnerDeployment creation methods to split a ConcurrencyLimitConfig into and int and ConcurrencyOption. This is also done to the CLI deploy to support the prefect.yaml

Related: #14934

Checklist

  • This pull request references any related issue by including "closes <link to issue>"
    • If no issue exists and your change is not a small fix, please create an issue first.
  • If this pull request adds new functionality, it includes unit tests that cover the changes
  • If this pull request removes docs files, it includes redirect settings in mint.json.
  • If this pull request adds functions or classes, it includes helpful docstrings.

Copy link

codspeed-hq bot commented Sep 18, 2024

CodSpeed Performance Report

Merging #15425 will not alter performance

Comparing jean/oss-5398-update-deployment-apis-to-take-an-object-for (524ea04) with main (a09aacc)

Summary

✅ 3 untouched benchmarks

@jeanluciano jeanluciano marked this pull request as ready for review September 18, 2024 20:22
Copy link
Member

@desertaxle desertaxle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall! I left a couple of nits on typing and dictionary key access.

src/prefect/cli/deploy.py Outdated Show resolved Hide resolved
src/prefect/deployments/runner.py Outdated Show resolved Hide resolved
src/prefect/deployments/runner.py Outdated Show resolved Hide resolved
src/prefect/deployments/runner.py Outdated Show resolved Hide resolved
src/prefect/flows.py Outdated Show resolved Hide resolved
src/prefect/flows.py Outdated Show resolved Hide resolved
src/prefect/runner/runner.py Outdated Show resolved Hide resolved
Copy link
Member

@desertaxle desertaxle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jeanluciano jeanluciano merged commit 54f6d06 into main Sep 19, 2024
30 checks passed
@jeanluciano jeanluciano deleted the jean/oss-5398-update-deployment-apis-to-take-an-object-for branch September 19, 2024 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants