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

Scedule Previous method implementation #298

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vincentmrg
Copy link

Hello,
I added the Previous method to the Schedule interface, as well as the implementation of this method to SpecSchedule. This method works on the same principle as the Next () method but on the other hand allows you to deduce the previous date where the schedule would have been activated.

This method is useful to us in order to define a list of dynamic configurations scheduled by cron and to deduce which configuration to use when applying these configurations.

@thinkerbot
Copy link

Interestingly enough I just went looking for a Previous method and was happy to see this PR. My use case is trying to identify a time interval defined by a specific time and a cron schedule.

@omaskery
Copy link

omaskery commented May 23, 2020

Hi, this PR looks perfect for my current problem. Is there anything I can do to help move this PR along? I've had a quick look at the changes and it all looks sane and tested :)

Edit: bad news @thinkerbot, @vincentmrg, it looks like a previous, almost identical PR was denied last year as being a use case outside the scope of this library - which is fair enough: https://github.com/robfig/cron/pull/216/files

I suspect there's probably room in the world for somebody to factor out the underlying cron schedule expression parsing and "next/prev" calculation, but this library (more focused on running jobs looking forward) probably isn't that place.

@vincentmrg
Copy link
Author

vincentmrg commented May 24, 2020

@omaskery Yep, I saw that a little late. As I need it, I still opened this PR.
@robfig free to you to accept or not this PR, it seems that some people find it useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants