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.

Sign up for GitHub

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

Jump to bottom

Add db-scheduler implementation of the Event Scheduler and Deadline Manager #2727

Merged
merged 28 commits into from Jun 1, 2023
Merged

Add db-scheduler implementation of the Event Scheduler and Deadline Manager #2727

merged 28 commits into from Jun 1, 2023

Conversation

gklijs
Copy link
Contributor

@gklijs gklijs commented May 23, 2023 •
edited

fixes issues/2724

@gklijs gklijs added Type: Enhancement Use to signal an issue enhances an already existing feature of the project. Priority 3: Could Low priority. Issues that are nice to have but have a straightforward workaround. Status: In Progress Use to signal this issue is actively worked on. labels May 23, 2023
@gklijs gklijs added this to the Release 4.8.0 milestone May 23, 2023
@gklijs gklijs self-assigned this May 23, 2023
@gklijs gklijs changed the title Add DbScheduler event scheduler. Add db-scheduler implementation of the Event Scheduler and Deadline Manager May 23, 2023
@gklijs gklijs marked this pull request as ready for review May 24, 2023 15:04
CodeDrivenMitch
CodeDrivenMitch requested changes May 26, 2023
View reviewed changes
Copy link
Member

@CodeDrivenMitch CodeDrivenMitch left a comment

Choose a reason for hiding this comment

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

Looks good, but I have some small requests for changes, and some concerns

pom.xml Show resolved Hide resolved
.../src/main/java/org/axonframework/springboot/autoconfig/AxonDbSchedulerAutoConfiguration.java Outdated Show resolved Hide resolved
messaging/src/main/java/org/axonframework/deadline/dbscheduler/DbSchedulerDeadlineManager.java Show resolved Hide resolved
messaging/src/main/java/org/axonframework/deadline/dbscheduler/DbSchedulerDeadlineManager.java Outdated Show resolved Hide resolved
messaging/src/main/java/org/axonframework/deadline/dbscheduler/DbSchedulerDeadlineManager.java Outdated Show resolved Hide resolved
...n/java/org/axonframework/eventhandling/scheduling/dbscheduler/DbSchedulerEventScheduler.java Outdated Show resolved Hide resolved
...in/java/org/axonframework/eventhandling/scheduling/dbscheduler/DbSchedulerScheduleToken.java Show resolved Hide resolved
@gklijs
Add a more compact pojo's for db-scheduler integration, which are use…
102e7c8 
…d by default. Users can also choose the more verbose human-readable pojo's if they want to.
smcvb
smcvb requested changes May 30, 2023
View reviewed changes
Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

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

Lot's of boring comments, but also some pointers we need to discuss before approval. Other than that, I think you've wiped this up very quickly: awesome!!

messaging/src/test/java/org/axonframework/utils/DbSchedulerTestUtil.java Outdated Show resolved Hide resolved
...g/src/main/java/org/axonframework/deadline/dbscheduler/DbSchedulerBinaryDeadlineDetails.java Outdated Show resolved Hide resolved
...g/src/main/java/org/axonframework/deadline/dbscheduler/DbSchedulerBinaryDeadlineDetails.java Outdated Show resolved Hide resolved
...g/src/main/java/org/axonframework/deadline/dbscheduler/DbSchedulerBinaryDeadlineDetails.java Outdated Show resolved Hide resolved
...rg/axonframework/deadline/dbscheduler/DbSchedulerBinaryDeadlineDetailsSerializationTest.java Outdated Show resolved Hide resolved
.../axonframework/eventhandling/scheduling/dbscheduler/BinaryDbSchedulerEventSchedulerTest.java Show resolved Hide resolved
.../src/main/java/org/axonframework/springboot/autoconfig/AxonDbSchedulerAutoConfiguration.java Outdated Show resolved Hide resolved
.../src/main/java/org/axonframework/springboot/autoconfig/AxonDbSchedulerAutoConfiguration.java Outdated Show resolved Hide resolved
.../src/main/java/org/axonframework/springboot/autoconfig/AxonDbSchedulerAutoConfiguration.java Outdated Show resolved Hide resolved
...axonframework/eventhandling/scheduling/dbscheduler/DbSchedulerEventSchedulerBuilderTest.java Outdated Show resolved Hide resolved
gklijs and others added 5 commits May 30, 2023 13:27
@gklijs @smcvb
Update messaging/src/test/java/org/axonframework/utils/DbSchedulerTes…
5c0f834 
…tUtil.java

Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
@gklijs @smcvb
Update messaging/src/main/java/org/axonframework/deadline/dbscheduler…
e15495d 
…/DbSchedulerBinaryDeadlineDetails.java

Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
@gklijs @smcvb
Update messaging/src/main/java/org/axonframework/deadline/dbscheduler…
dca44dd 
…/DbSchedulerBinaryDeadlineDetails.java

Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
@gklijs @smcvb
Update messaging/src/main/java/org/axonframework/deadline/dbscheduler…
37ceedb 
…/DbSchedulerBinaryDeadlineDetails.java

Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
@gklijs @smcvb
Update messaging/src/test/java/org/axonframework/eventhandling/schedu…
c84f634 
…ling/dbscheduler/DbSchedulerEventSchedulerBuilderTest.java

Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
gklijs and others added 10 commits May 30, 2023 13:29
@gklijs @smcvb
Update messaging/src/main/java/org/axonframework/deadline/dbscheduler…
cf81157 
…/DbSchedulerDeadlineToken.java

Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
@gklijs @smcvb
Update messaging/src/main/java/org/axonframework/deadline/dbscheduler…
35eb206 
…/DbSchedulerHumanReadableDeadlineDetails.java

Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
@gklijs @smcvb
Update messaging/src/test/java/org/axonframework/deadline/dbscheduler…
c0b57bb 
…/DbScheduledDeadlineTokenTest.java

Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
@gklijs @smcvb
Update messaging/src/main/java/org/axonframework/deadline/dbscheduler…
9572487 
…/DbSchedulerHumanReadableDeadlineDetails.java

Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
@gklijs @smcvb
Update messaging/src/main/java/org/axonframework/deadline/dbscheduler…
bb7a6e5 
…/DbSchedulerBinaryDeadlineDetails.java

Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
@gklijs @smcvb
Update messaging/src/main/java/org/axonframework/eventhandling/schedu…
b6d085b 
…ling/dbscheduler/DbSchedulerScheduleToken.java

Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
@gklijs @smcvb
Update messaging/src/main/java/org/axonframework/deadline/dbscheduler…
160fc38 
…/DbSchedulerDeadlineManager.java

Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
@gklijs @smcvb
Update messaging/src/main/java/org/axonframework/deadline/dbscheduler…
1f96d15 
…/DeadlineManagerNotSetException.java

Co-authored-by: Steven van Beelen <steven.vanbeelen@axoniq.io>
@gklijs
Fixed most of the review comments.
c418f28
@gklijs
Optionally start the scheduler using the Lifecycle. With Spring Boo…
6e1c3f1 
…t we leave it to the db scheduler starter to start the scheduler.
@gklijs gklijs requested a review from smcvb May 30, 2023 15:03
@gklijs
Change the parent of the exception to a generic AxonException as th…
42890b8 
…ere might be multiple causes of the exception.
smcvb
smcvb approved these changes Jun 1, 2023
View reviewed changes
Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

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

My main concerns have been tackeled, hence I am approving this pull request!
I do have a number of details left for discussion. However, I don't foresee any of those the revert my opinion.

...g/src/main/java/org/axonframework/deadline/dbscheduler/DbSchedulerBinaryDeadlineDetails.java Outdated Show resolved Hide resolved
...ain/java/org/axonframework/deadline/dbscheduler/DbSchedulerHumanReadableDeadlineDetails.java Outdated Show resolved Hide resolved
.../java/org/axonframework/eventhandling/scheduling/dbscheduler/DbSchedulerBinaryEventData.java Outdated Show resolved Hide resolved
...rg/axonframework/eventhandling/scheduling/dbscheduler/DbSchedulerHumanReadableEventData.java Outdated Show resolved Hide resolved
messaging/src/main/java/org/axonframework/deadline/dbscheduler/DbSchedulerDeadlineToken.java Outdated Show resolved Hide resolved
.../axonframework/eventhandling/scheduling/dbscheduler/BinaryDbSchedulerEventSchedulerTest.java Show resolved Hide resolved
...amework/eventhandling/scheduling/dbscheduler/HumanReadableDbSchedulerEventSchedulerTest.java Show resolved Hide resolved
.../src/main/java/org/axonframework/springboot/autoconfig/AxonDbSchedulerAutoConfiguration.java Show resolved Hide resolved
...n/java/org/axonframework/eventhandling/scheduling/dbscheduler/DbSchedulerEventScheduler.java Outdated Show resolved Hide resolved
messaging/src/main/java/org/axonframework/deadline/dbscheduler/DbSchedulerDeadlineManager.java Show resolved Hide resolved
smcvb
smcvb approved these changes Jun 1, 2023
View reviewed changes
Copy link
Member

@smcvb smcvb left a comment

Choose a reason for hiding this comment

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

My main concerns have been tackeled, hence I am approving this pull request!
I do have a number of details left for discussion. However, I don't foresee any of those the revert my opinion.

gklijs and others added 3 commits June 1, 2023 12:28
@sonarcloud
Copy link

sonarcloud bot commented Jun 1, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

84.8% 84.8% Coverage
7.2% 7.2% Duplication

@gklijs gklijs merged commit 23eec0c into master Jun 1, 2023
6 of 8 checks passed
@gklijs gklijs added Status: Resolved Use to signal that work on this issue is done. and removed Status: In Progress Use to signal this issue is actively worked on. labels Jun 1, 2023
@smcvb smcvb deleted the issues/2724 branch June 2, 2023 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kagkarlsson kagkarlsson kagkarlsson left review comments

@smcvb smcvb smcvb approved these changes

@CodeDrivenMitch CodeDrivenMitch Awaiting requested review from CodeDrivenMitch

Assignees

@gklijs gklijs

Labels
Priority 3: Could Low priority. Issues that are nice to have but have a straightforward workaround. Status: Resolved Use to signal that work on this issue is done. Type: Enhancement Use to signal an issue enhances an already existing feature of the project.
Projects
None yet
Milestone
Release 4.8.0
Development

Successfully merging this pull request may close these issues.

Add db-scheduler implementation of the Event Scheduler and Deadline Manager
4 participants
@gklijs @kagkarlsson @CodeDrivenMitch @smcvb