MichaelXavier / cron

Cron data structure and parser for Haskell
Other
53 stars 33 forks source link

Interpreting the schedule in a certain time zone #23

Closed bdesham closed 8 years ago

bdesham commented 9 years ago

My server is set to UTC. I have a haskell-cron application that should interpret its times in U.S. Eastern Standard Time (UTC-5) instead—for example, 0 0 * * * would specify a task that should run daily at 5:00 AM UTC. Is there a straightforward way to do this?

(I tried playing around in GHCi with the TZ environment variable set to US/Eastern, but the times were still interpreted in UTC.)

MichaelXavier commented 9 years ago

So the good news is the actual schedule types in cron don't really understand or care about time zones. The rubber meets the road at scheduleMatches :: CronSchedule -> UTCTime -> Bool. There's nothing inherent about that function that really needs it to be UTCTime. It just needs a Day value and a DiffTime or TimeOfDay. Maybe the way to think about this is that any CronSchedule should always be interpreted based on the time zone of its input, meaning you could apply the same CronSchedule to a UTCTime as a LocalTime. In that case, maybe a good solution would be to create a CronTime typeclass with instances for UTCTime, LocalTime, and ZonedTime. scheduleMatches would simply become more general and the few instances that call getCurrentTime for a UTCTime could instead take a CronTime t => IO t. The latter would probably be a breaking change.

Thoughts? I'd also like to /cc in possible users of cron @andrewthad @peti @albertov for comment.

albertov commented 9 years ago

I think that it's a good idea to implement this although my use cases (for the moment) don't need it since I always use UTCTimes.

I'm not user of the scheduling engine but after reading it's code I think that it could be made to work with any time data type that has a CronTime instance if this typeclass looks like this:

class CronTime t where
  scheduleMatches :: CronSchedule t -> t -> Bool
  currentTime :: IO t

instance CronTime UTCTime where
  scheduleMatches = ... -- currentImplementation
  currentTime = getCurrentTime

Notice that CronSchedule now needs a phantom type to carry information on how it should be interpreted, which I think is semantically correct. This is a breaking change though but shouldn't be too problematic to fix in client code by changing CronSchedule to CronSchedule UTCTime to get the current behaviour and should be correctly inferred if a specific time type is passed to scheduleMatches.

The scheduling engine functions will need to change its signatures to add the CronTime constraint and the Job, Jobs, Schdule and ScheduleT types should propagate the phantom type down to the CronSchedule field of a Job.

albertov commented 9 years ago

I've implemented this concept on #24

albertov commented 9 years ago

I'm thinking that this might also be implemented by adding a tz :: TimeZone field to CronSchedule. This will probably be simpler, more flexible (since it won't depend on the environment's TZ) and will not break backwards compatibility too badly (unless the CronSchedule constructor is used)

MichaelXavier commented 9 years ago

@albertov the part I'm concerned about is I can't find any way for real cron tabs to specify their time zone. They use the running user's time zone configuration and it is never specified in the file. Another way to think about it is you can take a crontab in GMT-7, take it to GMT+7 and run it and it will use the time supplied by the system with no modification needed to the file. I think whatever solution we come up with, the user should have an easy path to saying "I don't care about the time zone, just evaluate my schedule with respect to the times I give". I'm not sure if we need much more beyond that. You can even keep the same API for the scheduleEngine that confines the time type to UTCTime and uses getCurrentTime, but has a more general version that takes an IO t.

The user could totally create a wrapper over CronSchedule with a value for the time zone and implement their own check to error out if there's a mismatch, I just don't know if its a common enough use case that we should put it in the library itself.

MichaelXavier commented 8 years ago

I'm going to close this ticket and have time zones be a userland concern. Feel free to reopen if there is a compelling reason why this path won't be suitable.

ip1981 commented 6 years ago

By the way, Jenkins supports timezones in its cron specification, like TZ=... 0 * * * *.