-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add schedule task func #22
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: LeoTurnell-Ritson <[email protected]>
Co-authored-by: LeoTurnell-Ritson <[email protected]>
Co-authored-by: LeoTurnell-Ritson <[email protected]>
Co-authored-by: LeoTurnell-Ritson <[email protected]>
Co-authored-by: LeoTurnell-Ritson <[email protected]>
Co-authored-by: LeoTurnell-Ritson <[email protected]>
Add Scheduled task by hour and day
test file for register
Co-authored-by: LeoTurnell-Ritson <[email protected]>
Co-authored-by: LeoTurnell-Ritson <[email protected]>
Co-authored-by: LeoTurnell-Ritson <[email protected]>
Co-authored-by: LeoTurnell-Ritson <[email protected]>
Co-authored-by: LeoTurnell-Ritson <[email protected]>
…-schedule-task-func
if task.interval_minutes: | ||
add_task(task.execute, task.interval_minutes, next_run_time=soon if task.onstart else None) | ||
else: | ||
add_day_task(task.execute, task.day, task.hour, next_run_time=soon if task.onstart else None) |
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 don't think a day task should run on start. I think the paradigm implies that it will only run at the specified day/time.
last_timestamp = models.DateTimeField(null=True, blank=True) | ||
last_success = models.BooleanField(null=True, blank=True) | ||
last_runtime = models.FloatField(null=True, blank=True) | ||
day = models.CharField(max_length=3, null=True, blank=True) # Day of the week ('mon', 'tue', etc.) |
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.
Can you validate this with choices? https://docs.djangoproject.com/en/4.2/ref/models/fields/#choices
validators=[MinValueValidator(1), MaxValueValidator(1440)], | ||
null=True, | ||
blank=True | ||
) # 1 min to 24 hours | ||
|
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.
Can you add some validation that says you must either have day+hour OR interval_minutes?
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.
Looks good, with a few comments.
No description provided.