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

add new concurrency config to consolidate concurrency settings #26931

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

prha
Copy link
Member

@prha prha commented Jan 8, 2025

Summary & Motivation

This PR enables setting run concurrency settings from the run_queue configuration block into the concurrency deployment settings block.

This is to simplify concepts from a configuration standpoint.

The hypothesis is that most people will not need to even know about the run coordinator, and instead will only interact with this concurrency section.

How I Tested These Changes

BK

Changelog

Moves run queue configuration from its standalone deployment setting into the concurrency deployment setting, along with new settings for pools.

@prha prha force-pushed the prha/concurrency_config branch from 12b7103 to f71c7ea Compare January 8, 2025 04:31
@prha prha changed the base branch from master to prha/hoist_run_queue_config January 8, 2025 04:31
@prha prha changed the title Prha/concurrency config add new concurrency config to consolidate concurrency settings Jan 8, 2025
@prha prha force-pushed the prha/hoist_run_queue_config branch from 13f2c3c to 4710c64 Compare January 9, 2025 00:26
@prha prha force-pushed the prha/concurrency_config branch from f71c7ea to f572156 Compare January 9, 2025 00:26
@prha prha force-pushed the prha/hoist_run_queue_config branch from 4710c64 to 2ce911a Compare January 9, 2025 03:43
@prha prha force-pushed the prha/concurrency_config branch from f572156 to 8cfee9c Compare January 9, 2025 03:43
@prha prha force-pushed the prha/hoist_run_queue_config branch from 2ce911a to e5e8022 Compare January 9, 2025 03:58
@prha prha force-pushed the prha/concurrency_config branch from 8cfee9c to 152beca Compare January 9, 2025 03:58
@prha prha marked this pull request as ready for review January 9, 2025 18:26
@prha prha force-pushed the prha/hoist_run_queue_config branch from e5e8022 to 32eec84 Compare January 10, 2025 01:17
@prha prha force-pushed the prha/concurrency_config branch from 152beca to 068a084 Compare January 10, 2025 01:17
@prha prha force-pushed the prha/hoist_run_queue_config branch from 32eec84 to 3ba911e Compare January 10, 2025 23:23
@prha prha force-pushed the prha/concurrency_config branch from 068a084 to 6927f22 Compare January 10, 2025 23:23
@prha prha requested a review from OwenKephart January 14, 2025 19:12
@prha prha force-pushed the prha/concurrency_config branch from bf19552 to 17ed4d9 Compare January 16, 2025 01:10
@prha prha force-pushed the prha/hoist_run_queue_config branch from 253141f to 24f23d7 Compare January 16, 2025 20:23
@prha prha force-pushed the prha/concurrency_config branch from 17ed4d9 to 6a116d5 Compare January 16, 2025 20:23
@prha prha force-pushed the prha/hoist_run_queue_config branch from 24f23d7 to 16acc11 Compare January 17, 2025 01:08
@prha prha force-pushed the prha/concurrency_config branch from 6a116d5 to 7309b15 Compare January 17, 2025 01:08
@prha prha force-pushed the prha/hoist_run_queue_config branch from 16acc11 to 7dae80e Compare January 17, 2025 02:27
@prha prha force-pushed the prha/concurrency_config branch from 7309b15 to 256d726 Compare January 17, 2025 02:27
@prha prha force-pushed the prha/hoist_run_queue_config branch from 7dae80e to 8649c64 Compare January 17, 2025 02:55
@prha prha force-pushed the prha/concurrency_config branch from 256d726 to 3efd17a Compare January 17, 2025 02:55
@prha prha force-pushed the prha/hoist_run_queue_config branch from 8649c64 to f1b7934 Compare January 17, 2025 22:29
@prha prha force-pushed the prha/concurrency_config branch from 3efd17a to 3d2b288 Compare January 17, 2025 22:30
@prha prha changed the base branch from prha/hoist_run_queue_config to prha/run_pools_graphql January 17, 2025 22:30
@prha prha requested a review from OwenKephart January 17, 2025 22:41
Copy link

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-9iallqt0f-elementl.vercel.app
https://prha-concurrency-config.core-storybook.dagster-docs.io

Built with commit 3d2b288.
This pull request is being automatically deployed with vercel-action

Comment on lines 236 to 249
def verify_config_match(config: Mapping[str, Any], path_a: Sequence[str], path_b: Sequence[str]):
value_a = pluck_config_value(config, path_a)
value_b = pluck_config_value(config, path_b)
if value_a is None or value_b is None:
return

if value_a != value_b:
path_a_str = " > ".join(path_a)
path_b_str = " > ".join(path_b)
raise DagsterInvalidConfigError(
f"Found `{value_a}` for `{path_a_str}` that conflicts with `{value_b}` for `{path_b_str}`.",
[],
None,
)
Copy link
Contributor

Choose a reason for hiding this comment

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

this can be deleted now

@prha prha force-pushed the prha/concurrency_config branch from 3d2b288 to e63a776 Compare January 20, 2025 00:27
@prha prha changed the base branch from prha/run_pools_graphql to prha/hoist_run_queue_config January 20, 2025 00:27
@dagster-io dagster-io deleted a comment from vercel bot Jan 20, 2025
@prha prha force-pushed the prha/hoist_run_queue_config branch from f1b7934 to be7ec05 Compare January 21, 2025 19:57
@prha prha force-pushed the prha/concurrency_config branch from e63a776 to 6dd295b Compare January 21, 2025 19:57
prha added a commit that referenced this pull request Jan 21, 2025
…ce (#26932)

## Summary & Motivation
For ease of use, #26931
consolidates concurrency settings (including queued run coordinator
settings) into a single deployment settings block.

This PR makes it so that run queue settings are now accessed off of the
instance, instead of off of the run coordinator.

The run coordinator is a standalone-configured module. This makes it
difficult to incorporate instance settings from other places.

## How I Tested These Changes
BK
Base automatically changed from prha/hoist_run_queue_config to master January 21, 2025 20:51
@prha prha force-pushed the prha/concurrency_config branch from 6dd295b to 9f65155 Compare January 22, 2025 01:57
@prha prha merged commit dcb9a52 into master Jan 22, 2025
5 checks passed
@prha prha deleted the prha/concurrency_config branch January 22, 2025 18:56
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