fix: Set the daily schedule to a specific time, rather than 24hr from start up #3645
Conversation
|
|
||
| def _scheduled_task_wrapper(callable): | ||
| try: | ||
| callable() | ||
| except Exception as e: | ||
| logger.error(f"Error in scheduled task func='{callable.__name__}': exception='{e}'") | ||
| logger.error("Error in scheduled task func='%s': exception='%s'", callable.__name__, e) |
|
|
||
|
|
||
| @repeat_every(minutes=MINUTES_DAY, wait_first=True, logger=logger) | ||
| @repeat_every(minutes=MINUTES_DAY, wait_first=False, logger=logger) |
There was a problem hiding this comment.
We already handle the schedule elsewhere, so we don't want to wait on the first call to run_daily
| target_time = (now + delta).replace(microsecond=0, second=0) | ||
| logger.info("Daily tasks scheduled for %s", str(target_time)) | ||
| wait_seconds = (target_time - now).total_seconds() | ||
| await asyncio.sleep(wait_seconds) |
There was a problem hiding this comment.
Here we sleep to the first scheduled time, which is why we don't want to "wait" on the first run_daily - this is where that happens.
| str(now), | ||
| daily_schedule_time, | ||
| ) | ||
| hour_target = int(daily_schedule_time.split(":")[0]) |
There was a problem hiding this comment.
Can we add a try/catch on the daily_schedule_time parsing? Maybe bump it to its own method? That way this doesn't fail if the string is malformed. We could probably default to midnight and log the exception:
try:
hour_target, minute_target = _parse_daily_schedule_time(daily_schedule_time)
except Exception as e:
logger.exception(f"Unable to parse {daily_schedule_time=})
hour_target = minute_target = 0There was a problem hiding this comment.
Updated. Test:
mealie | DEBUG 2024-05-26T13:20:41 - Current time is 2024-05-26 13:20:41.016770 and DAILY_SCHEDULE_TIME is ThisIsBadData
mealie | ERROR 2024-05-26T13:20:41 - Unable to parse daily_schedule_time='ThisIsBadData'
mealie | Traceback (most recent call last):
mealie | File "/app/mealie/services/scheduler/scheduler_service.py", line 39, in schedule_daily
mealie | hour_target, minute_target = _parse_daily_schedule_time(daily_schedule_time)
mealie | File "/app/mealie/services/scheduler/scheduler_service.py", line 60, in _parse_daily_schedule_time
mealie | hour_target = int(time.split(":")[0])
mealie | ValueError: invalid literal for int() with base 10: 'ThisIsBadData'
mealie | ERROR 2024-05-26T13:20:41 - Unable to parse daily_schedule_time='ThisIsBadData'
mealie | Traceback (most recent call last):
mealie | File "/app/mealie/services/scheduler/scheduler_service.py", line 39, in schedule_daily
mealie | hour_target, minute_target = _parse_daily_schedule_time(daily_schedule_time)
mealie | File "/app/mealie/services/scheduler/scheduler_service.py", line 60, in _parse_daily_schedule_time
mealie | hour_target = int(time.split(":")[0])
mealie | ValueError: invalid literal for int() with base 10: 'ThisIsBadData'
mealie | DEBUG 2024-05-26T13:20:41 - Hours until 10 and minutes until 25
mealie | INFO 2024-05-26T13:20:41 - Daily tasks scheduled for 2024-05-26 23:45:00
mealie | INFO 2024-05-26T13:20:41 - Application startup complete.
mealie | INFO 2024-05-26T13:20:41 - Uvicorn running on http://0.0.0.0:9000 (Press CTRL+C to quit)
| @@ -16,7 +16,8 @@ | |||
| | TZ | UTC | Must be set to get correct date/time on the server | | |||
| | ALLOW_SIGNUP<super>\*</super> | false | Allow user sign-up without token | | |||
| | LOG_CONFIG_OVERRIDE | | Override the config for logging with a custom path | | |||
| | LOG_LEVEL | info | logging level configured | | |||
| | LOG_LEVEL | info | Logging level configured | | |||
| | DAILY_SCHEDULE_TIME | 23:45 | The time of day to run the daily tasks. | | |||
There was a problem hiding this comment.
Can we extend the last pipe out so it lines up? Just for my own sanity, I don't think it actually matters
There was a problem hiding this comment.
I'd rather not try to be pedantic about this .. that whole table structure is a bit of a mess, and any change made to try and fix it now will just get "broken" in future. There's a ton of inconsistency in there already.
There was a problem hiding this comment.
Huh, maybe it's just GitHub then, it seems to be formatted correctly to me:
But whatever it really doesn't matter anyway
There was a problem hiding this comment.
No, you're correct. VSCode was completely distorting it for me 🤦
Fixed.
What type of PR is this?
What this PR does / why we need it:
The previous logic for running daily tasks was that at container start it would NOT run, but sleep for 24hrs and then run. For people (like me) who might restart their container daily for backups, or who regularly update their image, this means they effectively never get the daily tasks run.
This change makes it so the daily tasks are scheduled to run at the next 23:45 (or other configured time). Meaning if a container is started at 22:00, it'll run 1:45 later (as opposed to 24:00 later).
This uses changes by @Grygon on #2497 as its basis.
Which issue(s) this PR fixes:
Maybe fixes #3454; it's the reason I noticed this.
Special notes for your reviewer:
Some comments will be added against the code.
Testing
Have done a bit on my prod system.
Realistic case:
Test case, showing it rolls to next day as needed:
Multi day Production run (note changes were implemented at 2024-05-24T20:37:46). This shows that the daily tasks are being run every 23:45 as expected despite, importantly, my container restarting at 1am each day.