Missing links to be filled in.
Thoughts on the small deployment 'setup' section here? Probably redundant with the other deployment pages
Details
Details
Diff Detail
Diff Detail
- Repository
- R1 dagster
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Event Timeline
Comment Actions
Generally looks good, few rambling thoughts.
as discussed offline i think since this is primarily instance config it should go on the Overviews (maybe alongside run launcher and executors, and rename that section to Runs or something? Not perfect. You could also call that section Run Coordinator (or Queueing Runs maybe) - somewhere in here we should probably explain succinctly what a Run Coordinator represents and how it relates to a Run Launcher since that could be confusing)
| docs/next/src/pages/deploying/limiting_run_concurrency.mdx | ||
|---|---|---|
| 18 ↗ | (On Diff #29066) | maybe contrast with the default run coordinator here, and explain what a run coordinator is? maybe make it explicit that with this coordinator, dagit just puts the runs in a queue, and its the daemon that does the actual queue management and launch You could also relate this to the run launcher - before a run goes into the run launcher, it first gets submitted to the run coordinator. |
| 20 ↗ | (On Diff #29066) | link to the instance overview? |
| 31 ↗ | (On Diff #29066) | this might be out of scope for the doc if this is moved out of the deploy section |
| 59 ↗ | (On Diff #29066) | You could remove 'The limits are restrictive', i think it's clear without. |
| 62 ↗ | (On Diff #29066) | maybe start with the default behavior and then contrast? "By default, runs will be removed from the queue in the order that they are added" |
| 64–65 ↗ | (On Diff #29066) | I see the goal here (explaining that it's not just a single fixed queue) but I found the explanation here somewhat hard to follow. It's kind of the flip side of the sentence above about 'if any limit would be exceeded by launching a run, then the run will stay queued.' right? This could go alongside that. "If no limit would be exceeded, then the run with the highest priorit will launch". And maybe make it explicit that if multiple runs are eligible to launch and have the same priority, the first one in will launch first? (Assuming that's true, I'm not sure it is) |
Comment Actions
feedback
| docs/next/src/pages/deploying/limiting_run_concurrency.mdx | ||
|---|---|---|
| 64–65 ↗ | (On Diff #29066) | Agreed it's a bit hard to follow. I think the critical bit is However, a run that is blocked by tag limits will not block runs submitted after it, maybe the example is overkill? |
| docs/next/src/pages/overview/pipeline-runs/limiting-run-concurrency.mdx | ||
|---|---|---|
| 39–42 | Maybe worth walking through the example a bit after stating it. "This run queue will only allow a maximum of 25 runs at once. Additionally, only 4 runs with the "database" tag equal to "redshift" can run at once, and at most 10 runs with the "dagster/backfill" key set to any value can run at once". | |