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

feat: Prev() function derives the previous time a SpecSchedule would have triggered #437

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

Conversation

juliev0
Copy link

@juliev0 juliev0 commented Feb 1, 2022

Issue 322 requested this function to derive the previous time that a SpecSchedule would have triggered.

I am open to moving the logic if you think I should.

Also, I am not sure if it works in the Sao Paolo case. I was having trouble understanding that one in SpecSchedule.Next() in the code and test.

@juliev0 juliev0 changed the title Prev() function derives the previous time a SpecSchedule would have triggered feat: Prev() function derives the previous time a SpecSchedule would have triggered Feb 1, 2022
@@ -174,6 +174,103 @@ WRAP:
return t.In(origLocation)
}

// Prev returns the previous time this schedule is activated, less than the given
// time. If no time can be found to satisfy the schedule, return the zero time.
func (s *SpecSchedule) Prev(t time.Time) time.Time {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it was wanted or not, but this function should be added to the Schedule interface for it to be accessible externally.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll add it to the interface and to the ConstantDelaySchedule struct.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, done

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There hasn't been any contribution to the repo since 2020 so let's hope that someone merges this! Thanks for implementing it though

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this change might be really beneficial for distributed systems (check if the previous job has been run). Is this project dead @robfig @juliev0?

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