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

[Scheduler] Add DateIntervalTrigger and DatePeriodTrigger #49739

Merged
merged 1 commit into from Mar 20, 2023

Conversation

fabpot
Copy link
Member

@fabpot fabpot commented Mar 19, 2023

Q A
Branch? 6.3
Bug fix? no
New feature? yes
Deprecations? no
Tickets n/a
License MIT
Doc PR -

The PeriodicalTrigger should only be used for a fixed period of time that can be converted to a number of seconds.
Using it for DatePeriod does not make sense as the number of seconds between 2 runs can differ.
This PR introduces 2 new triggers that behave correctly in such cases.

@carsonbot carsonbot added this to the 6.3 milestone Mar 19, 2023
@fabpot fabpot force-pushed the scheduler-simplification branch 2 times, most recently from 89af9e9 to ea3f6d3 Compare March 19, 2023 20:25
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

Is it worth keep PeriodicalTrigger then? Can't we create one using the new trigger?

@fabpot
Copy link
Member Author

fabpot commented Mar 19, 2023

Is it worth keep PeriodicalTrigger then? Can't we create one using the new trigger?

Maybe not.
@upyx Can you chime in here?

@fabpot
Copy link
Member Author

fabpot commented Mar 20, 2023

I've removed PeriodicalTrigger and ported some missing features in DateIntervalTrigger, so it supports the same scope.

@fabpot fabpot merged commit 8b8045b into symfony:6.3 Mar 20, 2023
3 of 7 checks passed
@fabpot fabpot deleted the scheduler-simplification branch March 20, 2023 08:56
@upyx
Copy link
Contributor

upyx commented Mar 21, 2023

@fabpot It doesn't work in real cases. Period iterator iterates over the all time points from the begginng. For example, some create a period from "2023-03-21" on every second. At "2024-03-12" the period will iterate over 31536000 poins on every worker start. It is the reason why I wrote different version.

Revert it, please.

@fabpot
Copy link
Member Author

fabpot commented Mar 25, 2023

@upyx I'm on it.

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

Successfully merging this pull request may close these issues.

None yet

5 participants