-
Notifications
You must be signed in to change notification settings - Fork 11
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: Support Startup Jobs #43 #70
Conversation
- restructured the NCronJobOptionsBuilder and JobOptionsBuilder so that the builder pattern delays the build() so we can finalize setting the options
4aefdc4
to
c18139e
Compare
c18139e
to
2022c70
Compare
…in with builder pattern
… for Observability
@@ -15,6 +15,10 @@ internal sealed record JobDefinition( | |||
|
|||
public CancellationToken CancellationToken { get; set; } | |||
|
|||
public bool IsOneTimeJob=> (CronExpression is null && RunAt != null) || IsStartupJob; | |||
public DateTimeOffset? RunAt { get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not particularly for this PR, but I added the same to another branch I'm working on for a bug fix related to instant jobs, so I just left it. I'll can remove it to not have to deal with merge issues.
@@ -0,0 +1,33 @@ | |||
namespace NCronJob; | |||
internal class StartupJobManager(JobRegistry jobRegistry) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For reasons of consistency, I would refrain from using primary constructors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, we should add an analyzer rule too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well technically I don't think there is one. I marked IDE0290
as none.
Not that I am absolutely against it, but I do think we need some shared guidelines and "enforce" them to some extent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So I am absolutely open to challenge the current coding standard/guidelines, especially if you feel that they are in your way and hinder productivity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the compactness of primary constructors. The only time they seem to get in the way is when using the source generated LoggerMessage
.
ccfcdad
to
49ca27e
Compare
Description
This PR introduces support for "startup jobs" and "one-time jobs" in the NCronJob library. These enhancements allow for tasks that need to be executed once during the application startup phase or at a specific moment, respectively. Key changes include:
Related Issue
Closes #43
Checklist