Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add time zone support for recurring jobs #327

Merged
merged 11 commits into from Mar 17, 2015
Merged

Add time zone support for recurring jobs #327

merged 11 commits into from Mar 17, 2015

Conversation

odinserj
Copy link
Member

This PR solves #160 and #270 by allowing to specify time zone for each recurring jobs. Each of the following jobs will fire at the same time (if Hawaii has no daylight saving time, I haven't google for this):

RecurringJob.AddOrUpdate("UTC",      () => Console.WriteLine("UTC"),       "15 18 * * *");
RecurringJob.AddOrUpdate("UTC2",     () => Console.WriteLine("UTC"),       "15 18 * * *", TimeZoneInfo.Utc);
RecurringJob.AddOrUpdate("Russian",  () => Console.WriteLine("Russian"),   "15 21 * * *", TimeZoneInfo.Local);
RecurringJob.AddOrUpdate("Hawaiian", () => Console.WriteLine("Hawaiian"),  "15 08 * * *", TimeZoneInfo.FindSystemTimeZoneById("Hawaiian Standard Time"));

image

Here are some facts about current implementation, they will be moved to the docs:

  • Default time zone is UTC. This may be confusing for users of Quartz.NET, Cron program in Linux, etc, that use local time by default. But switch to local time by default will introduce breaking changes, so it will be performed in version 2.0.
  • Windows Time Zone IDs are being used to store time zones. So there may be some issues when your Hangfire infrastructure contains both Windows and Linux. I thought about using NodaTime and its DateTimeProviders.Tzdb that uses IANA time zone ids, but it is overkill. Custom time zone providers may be implemented for Hangfire to handle this issue later.
  • Issues with Mono. TimeZoneInfo in Mono has some critical flaws in the latest stable Mono release at the time of this writing (2.10.8). TimeZoneInfo.Local has particular issues on the latest tested version of Mono. On Windows, TimeZoneInfo.Local throws TimeZoneNotFoundException. On Unix it returns a TimeZoneInfo with an Id of "Local", which isn't terribly useful (although it may contain the correct rules). – from http://nodatime.org/1.3.x/userguide/mono.html. This may be solved later, with custom time zone providers.

@odinserj odinserj added this to the 1.4.0 milestone Mar 17, 2015
@odinserj
Copy link
Member Author

Aaand unit test that is using Hawaiian Standard Time failed on Mono, because it does not know this time zone id (but knows Pacific/Honolulu, Pacific/Rarotonga, etc.).

Hangfire.Core.Tests.Server.RecurringJobSchedulerFacts.Execute_GetsInstance_InAGivenTimeZone [FAIL]
   System.TimeZoneNotFoundException : Exception of type 'System.TimeZoneNotFoundException' was thrown.
   Stack Trace:
     at System.TimeZoneInfo.FindSystemTimeZoneByFileName (System.String id, System.String filepath) [0x00000] in <filename unknown>:0 
     at System.TimeZoneInfo.FindSystemTimeZoneById (System.String id) [0x00000] in <filename unknown>:0 
     at Hangfire.Core.Tests.Server.RecurringJobSchedulerFacts.Execute_GetsInstance_InAGivenTimeZone () [0x00000] in <filename unknown>:0 
     at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (System.Reflection.MonoMethod,object,object[],System.Exception&)
     at System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) [0x00000] in <filename unknown>:0

odinserj added a commit that referenced this pull request Mar 17, 2015
Add time zone support for recurring jobs
@odinserj odinserj merged commit ef1d2ef into dev Mar 17, 2015
@odinserj odinserj deleted the recurring-timezones branch March 17, 2015 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant