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

Deprecate Cron.XXXInterval methods #1041

Closed
odinserj opened this issue Nov 6, 2017 · 2 comments
Closed

Deprecate Cron.XXXInterval methods #1041

odinserj opened this issue Nov 6, 2017 · 2 comments

Comments

@odinserj
Copy link
Member

odinserj commented Nov 6, 2017

Neither of these methods works correctly, when the specified interval is not a multiple of the corresponding field's range length (max-min). For example, when we are using Cron.HourInterval(10), we will not get the documented every 10 hours semantic, instead we'll get the following occurrences:

Nov 6, 00:00
Nov 6, 10:00
Nov 6, 20:00 
Nov 7, 00:00

Time interval between the last two entries is only 4 hours and violates what's written in the method's name and in its comments. Cron expressions don't allow to specify "every N" recurring intervals in general, and other methods should be used instead of confusing methods. These methods should be deprecated with the ObsoleteAttribute and removed in the next major version.

@almez
Copy link
Contributor

almez commented Dec 24, 2017

Hi,

Please find my first contribution to this repository.

Pull request #1084

Regards,

@odinserj odinserj removed this from the Hangfire 1.7.0 milestone Dec 25, 2017
odinserj pushed a commit that referenced this issue Mar 10, 2018
* #1041 Deprecate `Cron.XXXInterval` methods
* #1041 update the obsolete message
@odinserj odinserj added this to the Hangfire 1.7.0 milestone Mar 10, 2018
@crawdell
Copy link

crawdell commented Apr 5, 2018

I'd like to propose something I've done before to handle cron intervals. I needed a solution to "every 2 weeks" when using Ncrontab Advanced and so I extended the functionality to allow for "Xnn" to be added to a cron expression.

Example:
00 00 * * FRI X2

Without "X2" this would just be every Friday at midnight. But with X2 I would getnextoccurance 2 times and use the second value as the next run time which would be 2 weeks.

This can be useful for doing all kinds of new variations. It can also lead to confusing results sometimes if you don't think about your expression carefully but that's true of all cron expressions.

This I think helps fill in a gap that cron has and has been very useful for my scheduled jobs. Now that I'm using Hangfire I'll love to see this included.

I'm on the 1.7.0 beta and it works great. If this could somehow be added that would be very helpful. If not I might try my own version with the source either by extending Hangfire or more likely Cronos.

staviloglu added a commit to staviloglu/Hangfire that referenced this issue Nov 24, 2022
Updated MinuteInterval & HourInterval methods to make them available (not obselete) again.

Added defensive code against interval value to get rid of the problem described in HangfireIO#1041 

Also related:
HangfireIO#1054
HangfireIO#1779
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants