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

OrTrigger does not work with DateTriggers #427

Closed
DmitriiKolesnichenko opened this issue Apr 5, 2020 · 5 comments
Closed

OrTrigger does not work with DateTriggers #427

DmitriiKolesnichenko opened this issue Apr 5, 2020 · 5 comments
Labels
Milestone

Comments

@DmitriiKolesnichenko
Copy link

DmitriiKolesnichenko commented Apr 5, 2020

I create trigger OrTrigger(DateTrigger(date1), DateTrigger(date2)).
date1 < date2.

Expected Behavior

Trigger fire twice: in date1 and date2.

Current Behavior

Trigger fires once: in date1.

Detailed Description

It occurs because DateTrigger.get_next_fire_time() returns None if previous_fire_time is not None. If we use OrTrigger, we pass previous_fire_time to all included triggers. And all of them returns None after the first fire of OrTrigge. So it will never work multiple times.

@Laurel-rao
Copy link

put your OrTrigger code below please.

@agronholm
Copy link
Owner

put your OrTrigger code below please.

Not necessary; the analysis sounds correct. This indeed seems like a bug to me.

@agronholm agronholm added the bug label May 14, 2020
@agronholm
Copy link
Owner

I am currently working on the refactoring of the trigger system in APScheduler 4.0. The stateful nature of triggers there solves this problem since OrTrigger will store the trigger times produced by all the contained triggers.

@agronholm agronholm added this to the 4.0 milestone May 15, 2020
@agronholm
Copy link
Owner

Come to think of it, this is actually fixable even in v3.x if the logic is changed to only return None if the previous fire time is equal to or later than the run date of the trigger.

@agronholm
Copy link
Owner

Solved by the addition of stateful triggers (17a353f).

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

No branches or pull requests

3 participants