Page MenuHomePhabricator

Add execution_timezone field to schedules
ClosedPublic

Authored by dgibson on Thu, Oct 8, 5:40 PM.

Details

Summary

Allows individual schedules to run at a fixed timezone (overridding whatever timezone has been set on the instance).

Test Plan

BK

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

dgibson retitled this revision from Add execution_timestamp field to schedules to Add execution_timezone field to schedules.Thu, Oct 8, 5:43 PM
python_modules/dagster/dagster/core/definitions/decorators/schedule.py
513–517

Won't this be confusing in the playground and backfill UIs now? I feel like if they want their schedule to run every hour exactly, they should use UTC execution_timezone. But if they configure an hourly schedule in a DST timezone, the partitions should be local to their timezone and the behavior at DST boundaries should be documented.

python_modules/dagster/dagster/core/definitions/decorators/schedule.py
513–517

Made the partition names (and values) timezone-aware in https://dagster.phacility.com/D4761

Ditch the instance-level default timezone (going to tackle getting the partition names and values exactly right separately)

Take timezone into account when computing partition names/values

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

Maybe mark this argument on all the decorators with the new experimental feature flags

python_modules/dagster/dagster/scheduler/scheduler.py
212–215

Where do these show up again?

python_modules/dagster/dagster_tests/scheduler_tests/test_scheduler_timezones.py
111–113

Shouldn't all these tests now freeze the time in UTC?

145

The result would be the same even if this were 1 minute later right?

python_modules/dagster/dagster/scheduler/scheduler.py
212–215

just as output in the scheduler process

For 0.10.0 IMO we should require execution_timezone on all schedules, so this should become an invariant violation

python_modules/dagster/dagster_tests/scheduler_tests/test_scheduler_timezones.py
111–113

I don't think so no, there's no guarantee that the server running them will be in UTC

145

yeah, will update to make more clear

chefkiss

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

Is there some sort of built in type in datetime that we can use here to represent the timezone instead of a string?

This revision is now accepted and ready to land.Wed, Oct 14, 9:21 PM

fix some issues with calculating the partition times during DST transitions (can't trust relativedelta or timedelta, can trust pendulum.datetime.subtract and pendulum.range)

This revision was automatically updated to reflect the committed changes.