Page MenuHomePhabricator

Require that start_date be on a month/day boundary for monthly/daily schedules
ClosedPublic

Authored by dgibson on Oct 15 2020, 1:22 PM.

Details

Summary

Right now the start date determines what time gets passed in as a partition value to the run config, which might not be what people expect (like if you start a monthly schedule on the 5th of each month, we pass in a partition with the name as the month and the value as the 5th of each month). It also makes it harder to think about timezones and ensuring that we can map from tick time to partition time, although I don't *think* there's actually any bugs here.

Test Plan

New BK coverage

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

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 15 2020, 1:45 PM
Harbormaster failed remote builds in B19545: Diff 23744!
Harbormaster returned this revision to the author for changes because remote builds failed.Oct 15 2020, 2:43 PM
Harbormaster failed remote builds in B19549: Diff 23751!
Harbormaster returned this revision to the author for changes because remote builds failed.Oct 15 2020, 3:27 PM
Harbormaster failed remote builds in B19553: Diff 23756!
Harbormaster returned this revision to the author for changes because remote builds failed.Oct 15 2020, 4:17 PM
Harbormaster failed remote builds in B19560: Diff 23763!

Nice catch - this is a great fix. We should note this in the change log, because it has the potential to break some people's schedules/repositories.

python_modules/dagster/dagster/core/definitions/decorators/schedule.py
408

Nit: "at execute the schedule at a specified time in the day"? Same for the other messages.

It might be a good idea to have a tiny example right here in these error messages, because I don't think it's entirely intuitive, but an example would immediately clarify what's wrong.

Something along the lines of:

For example, if you want to have a schedule run at 3:00 AM EST starting Sept 23, your schedule definition would look like:

@daily_schedule(
    start_date=datetime.datetime(2020, 9, 23),
    execution_time=datetime.time(3, 0), 
    execution_timezone="US/EST" # or whatever it is
    ...
):
def my_schedule_definition(_):
   pass
This revision is now accepted and ready to land.Oct 18 2020, 4:49 PM
python_modules/dagster/dagster/core/definitions/decorators/schedule.py
408

Sounds good, will add the example (minus the timezones since that isn't ready yet until the new scheduler is documented and stuff)

Add more descriptive comments and examples

switch to a warning for a less breaking change