Page MenuHomePhabricator

Support preview configuration for dynamic schedule configs
ClosedPublic

Authored by sashank on Nov 12 2019, 8:03 PM.

Details

Summary

If a ScheduleDefinition uses a dynamic config (environment_config_fn), then Dagit would not properly load the config. This diff updates the gql resolvers to evaluation the config function if it exists to support the view configuration feature.

Caveat is that environment_config_fn can't have any side effects.

Test Plan

unit

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

sashank updated this revision to Diff 6474.Nov 12 2019, 8:03 PM
sashank created this revision.

up

alangenfeld added inline comments.
python_modules/dagster-graphql/dagster_graphql/schema/run_schedule.py
56–72

this is confusing me - what exactly is going on here?

python_modules/dagster-graphql/dagster_graphql_tests/graphql/setup_scheduler.py
39

dynamit

Harbormaster completed remote builds in B5209: Diff 6474.
sashank added inline comments.Nov 12 2019, 9:07 PM
python_modules/dagster-graphql/dagster_graphql/schema/run_schedule.py
56–72

For a ScheduleDefinition, you can either pass in a hardcoded environment_dict:

ScheduleDefinition(
  ...
  environment_dict={...}
)

Or a function that returns an environment dict:

ScheduleDefinition(
   ...
   environment_dict_fn={...}
)

We also construct a execution_params dict within ScheduleDefinition, where execution_params['environmentConfigData'] should be set to either the hardcoded environment_dict or the return value of environment_dict_fn.

Here, if schedule_def.environment_dict_fn is defined, we replace execution_params['environmentConfigData'] with the result of schedule_def.environment_dict_fn()

alangenfeld accepted this revision.Nov 12 2019, 9:19 PM

fix the typo then g2g

python_modules/dagster-graphql/dagster_graphql/schema/run_schedule.py
56–72

thanks, i misread some of this and thought it was coming off of graphene

This revision is now accepted and ready to land.Nov 12 2019, 9:19 PM
sashank updated this revision to Diff 6475.Nov 12 2019, 9:21 PM

Fix typo