Skip to content
This repository has been archived by the owner on Sep 18, 2021. It is now read-only.

Schedules to not take timezone into account #11

Closed
mbyrne00 opened this issue Dec 17, 2020 · 3 comments
Closed

Schedules to not take timezone into account #11

mbyrne00 opened this issue Dec 17, 2020 · 3 comments

Comments

@mbyrne00
Copy link

mbyrne00 commented Dec 17, 2020

Hi,

Regardless of what is entered into the timezone of a schedule, it seems to always kick off according to UTC.

The simplest fix is to change the following line

IF :GRAPHILE_SCHEDULER_SCHEMA.schedules_matches(v_schedule, v_next_check) THEN
to the following ...

FROM

  	IF :GRAPHILE_SCHEDULER_SCHEMA.schedules_matches(v_schedule, v_next_check) THEN

TO

  	IF :GRAPHILE_SCHEDULER_SCHEMA.schedules_matches(v_schedule, v_next_check  AT TIME ZONE v_schedule.timezone) THEN

It would probably be better if the schedules_matches function itself could be passed any timestamp and did that translation internally but my PG plsql is very rusty.

Once I did the fix the schedules kicked off at the correct time. The job entries still had the fireDate specified in UTC time but it was the correct equivalent time.

mbyrne00 pushed a commit to mondo-mob/graphile-scheduler that referenced this issue Dec 17, 2020
@davbeck
Copy link
Owner

davbeck commented Dec 17, 2020

Oh man that's a pretty big oversight. To be clear, with that change, it worked as expected?

@mbyrne00
Copy link
Author

mbyrne00 commented Dec 18, 2020

Hey @davbeck - yes that quick fix did the trick. I can take a look at fixing the schedules_matches function instead of the quickfix and submitting a PR but can I please ask that you publish a new version after merging PR? We've had to keep using our own npm releases because the other merged PRs weren't released.

Otherwise we'd be happy to join as contributors to help speed things up.

mbyrne00 pushed a commit to mondo-mob/graphile-scheduler that referenced this issue Dec 18, 2020
…tch dates using defined timezone

 - Revert previous "quick fix" that mutated existing migration
 - New migration to re-create function

 davbeck#11
mbyrne00 pushed a commit to mondo-mob/graphile-scheduler that referenced this issue Dec 18, 2020
mbyrne00 pushed a commit to mondo-mob/graphile-scheduler that referenced this issue Dec 18, 2020
mbyrne00 pushed a commit to mondo-mob/graphile-scheduler that referenced this issue Dec 18, 2020
mbyrne00 pushed a commit to mondo-mob/graphile-scheduler that referenced this issue Dec 18, 2020
@mbyrne00
Copy link
Author

mbyrne00 commented Dec 18, 2020

PR submitted ... tested locally with a bunch of crons and it fixes the issue. The PR does the update properly with a migration and fixes the schedules_matches function instead of the quick fix I did above. If you can please publish after merging that would be great.

@davbeck davbeck closed this as completed Dec 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants