Open
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR significantly refactors the EventBridge service in LocalStack, focusing on improving the job scheduling and event dispatching mechanisms.
- Introduced a new
Schedulerutility inutils/scheduler.pyfor more flexible and efficient task scheduling - Implemented
CronScheduledTaskandJobclasses inevents/scheduler.pyto support both cron and rate expressions - Refactored
EventsProviderinevents/provider.pyto use the newJobSchedulerfor handling scheduled rules - Added
EventDispatcherand its implementations inevents/dispatcher.pyfor improved event dispatching - Updated
put_rule,delete_rule,disable_rule, andenable_rulemethods to work with the new job scheduling system
5 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings
| if expression.startswith("rate"): | ||
| return parse_rate_expression(expression) | ||
|
|
||
| raise ValueError("Syntax error in expression") |
There was a problem hiding this comment.
style: Add more specific error message, e.g., 'Invalid schedule expression. Must start with "cron" or "rate".'
Comment on lines
+134
to
+135
| if task.deadline is None: | ||
| task.deadline = max(task.start or 0, time.time()) |
There was a problem hiding this comment.
style: Consider using 'or' instead of 'is None' for more concise code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
The key mechanism of the JobScheduler class was a 59.9 second wait event in a while true loop:
We have an event-based scheduler utility that could be used for the JobScheduler, which will also make things easier for our time simulation project.
So this PR is both about cleaning up tech debt and making the JobScheduler more flexible.
Changes