Page MenuHomeElementl

Create Scheduler GraphQL interface
ClosedPublic

Authored by sashank on Aug 13 2019, 9:22 AM.

Details

Reviewers
alangenfeld
Group Reviewers
Restricted Project
Commits
R1:021cc034dea5: Create Scheduler GraphQL interface
Test Plan

unit

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

alangenfeld added a subscriber: alangenfeld.
alangenfeld added inline comments.
js_modules/dagit/src/schema.graphql
305

wat

757

this seems accidental?

python_modules/dagster-graphql/dagster_graphql/implementation/scheduler/system_cron_scheduler.py
29
  • do a lil namedtuple, these random string key accessors strewn about seem dubious
  • probably rename _schedules to make it clear it holds these wrapper objs
90–98

actually i would consider just adding file_path as a property to RunSchedule that doesnt get exposed to graphql

This revision now requires changes to proceed.Aug 13 2019, 3:21 PM

Use namedtuple instead of dict to store schedule and file_path

Change type of execution_params to String

sashank marked an inline comment as done.

make graphql

sashank added inline comments.
python_modules/dagster-graphql/dagster_graphql/implementation/scheduler/system_cron_scheduler.py
90–98

I think it might be best to leave it to the Scheduler implementation, since other implementations won't won't need file paths, will need to store multiple file paths, or use a database, etc.

For ex: I'll need to add a path to the cron shell script for this scheduler.

sashank marked an inline comment as done.

Fix diff

Remove timestamp from schedule filenames, removing the need for a wrapper around schedules in SystemCronScheduler

alangenfeld added inline comments.
js_modules/dagit/src/schema.graphql
757

is String right here? we just send the json blob?

python_modules/dagster-graphql/dagster_graphql/schema/run_schedule.py
10–11

maybe use a string instead of 1? not sure how beneficial it actually is, depends where the value actually shows up

117

might be worth having a slightly different name since this returns a string that needs to get json decoded by the client

This revision is now accepted and ready to land.Aug 14 2019, 6:46 PM
sashank added inline comments.
js_modules/dagit/src/schema.graphql
757

I think so. I don't see a situation where you would want to get just a part of the execution params, and I don't see a way to make it query-able since the structure changes based on config.

Rename execution_params to execution_params_string in DauphinRunSchedule