Page MenuHomePhabricator

Store timestamp on ScheduleTickTable
ClosedPublic

Authored by sashank on Fri, Mar 20, 12:41 AM.

Details

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 created this revision.Fri, Mar 20, 12:41 AM
alangenfeld added inline comments.
python_modules/dagster/dagster/core/storage/schedules/schema.py
25–28

something odd about the three timestamp columns - what exactly is their relationship?

sashank added a reviewer: max.Fri, Mar 20, 5:51 PM
sashank updated this revision to Diff 10758.Fri, Mar 20, 5:54 PM

Add comment about time related columns

max added inline comments.Fri, Mar 20, 6:00 PM
python_modules/dagster/dagster/core/storage/schedules/sql_schedule_storage.py
88

this is right, if we wanted to be very scrupulous we might make the pytz dependency 2.7 only and in 3 do:
datetime.datetime.fromtimestamp(schedule_tick_data, datetime.timezone.utc)

python_modules/dagster/setup.py
90

then this would be conditional on python_version < 3

sashank updated this revision to Diff 10760.Fri, Mar 20, 7:34 PM

Conditionally use pytz or datetime.timezeone based on python version

max accepted this revision.Fri, Mar 20, 7:55 PM
This revision is now accepted and ready to land.Fri, Mar 20, 7:55 PM
This revision was automatically updated to reflect the committed changes.