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

Deployment concurrency limits/configurable collision strategy #2745

Merged

Conversation

collincchoy
Copy link
Contributor

This PR adds the ability to edit/update a deployment's deployment.concurrency_options.collision_strategy from the UI.

Screenshot 2024-09-20 at 2 39 43 PM

@collincchoy collincchoy requested a review from a team as a code owner September 20, 2024 18:45
Copy link
Collaborator

@pleek91 pleek91 left a comment

Choose a reason for hiding this comment

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

LGTM

@abrookins
Copy link
Contributor

I wonder if we can make the options more self-explanatory. 🤔 What if the user-facing text for the options was something like "Enqueue new runs" or "Cancel new runs"?

@collincchoy
Copy link
Contributor Author

collincchoy commented Sep 20, 2024

I wonder if we can make the options more self-explanatory. 🤔 What if the user-facing text for the options was something like "Enqueue new runs" or "Cancel new runs"?

It's a good question. I considered it but decided to lean on parity with the client-side python implementation to reduce confusion.

update: I think we can always update and make this clearer further down the line so i'm gonna go with getting the functionality merged in here and then if we get later feedback, we can always revisit - 🚀

@collincchoy collincchoy merged commit f3258f1 into main Sep 20, 2024
2 checks passed
@collincchoy collincchoy deleted the deployment-concurrency-limits/configurable-collision-strategy branch September 20, 2024 19:38
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.

3 participants