Page MenuHomePhabricator

Create ScheduleDefinition and dagster CLI scheduler commands
ClosedPublic

Authored by sashank on Mon, Aug 19, 11:11 PM.

Details

Summary

In this v1 implementation, we attach ScheduleDefinitions to a RepositoryDefinition.

many_events_daily_schedule = ScheduleDefinition(
    name="many_events_midnight_daily",
    cron_schedule="0 0 * * *",
    execution_params={
        "environmentConfigData": {"storage": {"filesystem": None}},
        "selector": {"name": "many_events", "solidSubset": None},
        "mode": "default",
    },
)

many_events_every_minute_schedule = ScheduleDefinition(
    name="many_events_midnight_every_minute",
    cron_schedule="* * * * *",
    execution_params={
        "environmentConfigData": {"storage": {"filesystem": None}},
        "selector": {"name": "many_events", "solidSubset": None},
        "mode": "default",
    },
)

return RepositoryDefinition(
    name='toys_repository',
    pipeline_defs=[
        many_events,
    ],
    schedule_defs=[many_events_daily_schedule, many_events_every_minute_schedule],
)

We can then interact with the ScheduleDefinitions using the dagster cli.

$ dagster schedules list
Repository toys_repository
**************************
Schedule: many_events_midnight_daily
Cron Schedule: 0 0 * * *
********************************************
Schedule: many_events_midnight_every_minute
Cron Schedule: * * * * *
$ dagster schedules list --verbose
Repository toys_repository
**************************
Schedule: many_events_midnight_daily
Cron Schedule: 0 0 * * *
Execution Params: {'environmentConfigData': {'storage': {'filesystem': None}}, 'selector': {'name': 'many_events', 'solidSubset': None}, 'mode': 'default'}
********************************************
Schedule: many_events_midnight_every_minute
Cron Schedule: * * * * *
Execution Params: {'environmentConfigData': {'storage': {'filesystem': None}}, 'selector': {'name': 'many_events', 'solidSubset': None}, 'mode': 'default'}
$ dagster schedules start many_events_midnight_daily
Started schedule many_events_midnight_daily with ID 2a0ebd26-da90-4a71-96df-f249f50a6498
$ dagster schedules list --running
Repository toys_repository
**************************
Schedule: many_events_midnight_daily [Running]
Cron Schedule: 0 0 * * *
$ dagster schedules end many_events_midnight_daily
Ended schedule many_events_midnight_daily with ID 2a0ebd26-da90-4a71-96df-f249f50a6498
$ dagster schedules list --running
Repository toys_repository
**************************
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 3832.Mon, Aug 19, 11:11 PM
sashank created this revision.

Rebase

sashank updated this revision to Diff 3838.Tue, Aug 20, 12:39 AM

Delete GraphQL queries for schedule create + delete, fix tests

sashank updated this revision to Diff 3841.Tue, Aug 20, 12:47 AM

Fix graphql errors

sashank updated this revision to Diff 3842.Tue, Aug 20, 1:07 AM

Move scheduler from dagster_graphql to dagster.core

sashank updated this revision to Diff 3847.Tue, Aug 20, 5:10 PM

Update paths

sashank updated this revision to Diff 3849.Tue, Aug 20, 5:22 PM

Remove schedules from toys, Update tests

sashank updated this revision to Diff 3850.Tue, Aug 20, 5:24 PM

Update documentation

Harbormaster completed remote builds in B3081: Diff 3850.
sashank retitled this revision from [WIP] Create ScheduleDefinition and dagster CLI scheduler commands to Create ScheduleDefinition and dagster CLI scheduler commands.Tue, Aug 20, 5:35 PM
sashank edited the summary of this revision. (Show Details)
sashank edited the test plan for this revision. (Show Details)
sashank added a reviewer: Restricted Project.
sashank edited the summary of this revision. (Show Details)
sashank edited the summary of this revision. (Show Details)
sashank updated this revision to Diff 3852.Tue, Aug 20, 5:54 PM
sashank edited the summary of this revision. (Show Details)

Improve error handling and messages

alangenfeld requested changes to this revision.Tue, Aug 20, 6:58 PM
alangenfeld added a subscriber: alangenfeld.

dagster schedule list
dagster schedule running

I think it might be better to do a more verbose list output and then also include flags, maybe one for even more verbosity and one for filtering running. Thoughts?

dagster schedule

what do you think about dagster schedules? reads a bit more naturally to me

python_modules/dagster-graphql/dagster_graphql/schema/roots.py
290–292

rename start_run_schedule / end_run_schedule

python_modules/dagster/dagster/cli/__init__.py
18

should this be gated behind the feature flag?

python_modules/dagster/dagster/cli/schedule.py
23

this is an issue with cron scheduler specifically right? we depend on repo.yaml for producing the .sh file? Might be good to put a bit more context here of why

162–164

im not sure the exact best place - but we should slam some [Experimental]s around here

python_modules/dagster/dagster/core/definitions/repository.py
26–37

I think we should introduce an experimental dict here to make it extra clear that this stuff is experimental

RepositoryDefinition(
  pipeline_defs=[pipe]
  experimental={
    schedule_defs=[SchedulDefinition(...)]
  }
)
53

the _cache suffix doesnt make as much sense here since we dont have the same problem as pipelines where they can be lazily evaluated via a function

This revision now requires changes to proceed.Tue, Aug 20, 6:58 PM
sashank updated this revision to Diff 3862.Tue, Aug 20, 8:51 PM
sashank marked 5 inline comments as done.

Feedback

sashank updated this revision to Diff 3863.Tue, Aug 20, 8:53 PM

Rename start + end schedule mutations

sashank edited the summary of this revision. (Show Details)Tue, Aug 20, 8:57 PM
sashank added inline comments.Tue, Aug 20, 9:08 PM
python_modules/dagster/dagster/cli/__init__.py
18

yup, put it behind the 'scheduler' flag

python_modules/dagster/dagster/cli/schedule.py
162–164

Added [Experimental] to all the command descriptions

sashank updated this revision to Diff 3868.Tue, Aug 20, 9:11 PM

Bug fix

schrockn added inline comments.
python_modules/dagster/dagster/cli/pipeline.py
103 ↗(On Diff #3863)

kill

python_modules/dagster/dagster/core/definitions/repository.py
35

stick to double or single quotes

54

can you just check dictionary keys? set seems duplicative

python_modules/dagster/dagster/core/definitions/schedule.py
5

consider inheriting from named tuple

schrockn accepted this revision.Tue, Aug 20, 9:17 PM

This is looking good. Per discussion in follow ons let's extract dagster-cron (or whatever we call it) into its own module so that the core library is not tied to the cron scheduler.

Also please heed my final comments. They appear out-of-date because the diff was updated while I was reviewing.

Great progress!

sashank updated this revision to Diff 3870.Tue, Aug 20, 9:22 PM
sashank marked 2 inline comments as done.

Remove _schedule_names, change ScheduleDefinition to namedtuple, fix nits

sashank updated this revision to Diff 3871.Tue, Aug 20, 9:27 PM

Rerun tests

sashank updated this revision to Diff 3872.Tue, Aug 20, 9:31 PM

make graphql

natekupp accepted this revision.Tue, Aug 20, 10:16 PM
This revision was not accepted when it landed; it landed in state Needs Review.Tue, Aug 20, 10:20 PM
This revision was automatically updated to reflect the committed changes.