Page MenuHomePhabricator

load dagit settings for queueing execution manager from dagster.yaml

Authored by prha on Jan 8 2020, 6:44 PM.


Test Plan

configured dagster.yaml with:

  module: dagster_graphql.launcher
  class: RemoteDagitRunLauncher
    address: http://localhost:3333
    max_concurrent_runs: 2

and ran jobs scheduled through both dagit and the backfill cli

Diff Detail

R1 dagster
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

prha created this revision.Jan 8 2020, 6:44 PM
prha updated this revision to Diff 8453.Jan 8 2020, 6:53 PM

fix missed callsite

prha edited the test plan for this revision. (Show Details)Jan 8 2020, 8:11 PM
prha added reviewers: alangenfeld, schrockn.

hm this seems a little odd - why are dagit settings flowing through next to the instance - shouldn't be part of the instance?

prha added a comment.Jan 9 2020, 12:44 AM

that's a good question.... I always thought of dagit as being orthogonal to the dagster instance, but it makes things a lot simpler just to have this be on the instance config

alangenfeld requested changes to this revision.Jan 9 2020, 3:50 PM

I think it makes sense to opt for the simpler / lower code surface area approach for now and we can refactor it out later when we have a clearer sense of things

This revision now requires changes to proceed.Jan 9 2020, 3:50 PM
prha updated this revision to Diff 8517.Jan 9 2020, 7:13 PM

Adds a dagit settings section to the instance config... This keeps the dagit settings as a dictionary instead of a configurable class.

    max_concurrent_runs: 2

This is because the execution manager requires the instance itself, so cannot be a fully hydrated
configurable class

alangenfeld accepted this revision.Jan 9 2020, 10:50 PM

Update [1] then good to go I think



This revision is now accepted and ready to land.Jan 9 2020, 10:50 PM
prha updated this revision to Diff 8574.Jan 10 2020, 12:20 AM

add dagit settings info to dagit info panel

prha updated this revision to Diff 8576.Jan 10 2020, 12:28 AM