From 06f8a5095650d0fdb69d9dacbaa5092260a03495 Mon Sep 17 00:00:00 2001 From: Oliver <oliver.henry.walters@gmail.com> Date: Thu, 9 Mar 2023 17:46:07 +1100 Subject: [PATCH] Update task limiting (#4472) * Add new setting to control how often we check for new version * Improved names for settings * Fix bug in existing backup task * Refactor backup_task functino - Add a helper function for running tasks at multi-day intervals * Refactoring * Add unit tests for new helper function * Add multi-day holdoff to "check for updates" * Allow initial attempt * Add missing return * Fixes for unit test --- InvenTree/InvenTree/tasks.py | 159 ++++++++++++------ InvenTree/InvenTree/tests.py | 53 ++++++ InvenTree/common/models.py | 34 ++-- .../templates/InvenTree/settings/global.html | 1 + 4 files changed, 186 insertions(+), 61 deletions(-) diff --git a/InvenTree/InvenTree/tasks.py b/InvenTree/InvenTree/tasks.py index 3e39076ca3..4831a7fcd4 100644 --- a/InvenTree/InvenTree/tasks.py +++ b/InvenTree/InvenTree/tasks.py @@ -74,6 +74,93 @@ def raise_warning(msg): warnings.warn(msg) +def check_daily_holdoff(task_name: str, n_days: int = 1) -> bool: + """Check if a periodic task should be run, based on the provided setting name. + + Arguments: + task_name: The name of the task being run, e.g. 'dummy_task' + setting_name: The name of the global setting, e.g. 'INVENTREE_DUMMY_TASK_INTERVAL' + + Returns: + bool: If the task should be run *now*, or wait another day + + This function will determine if the task should be run *today*, + based on when it was last run, or if we have a record of it running at all. + + Note that this function creates some *hidden* global settings (designated with the _ prefix), + which are used to keep a running track of when the particular task was was last run. + """ + + from common.models import InvenTreeSetting + + if n_days <= 0: + logger.info(f"Specified interval for task '{task_name}' < 1 - task will not run") + return False + + # Sleep a random number of seconds to prevent worker conflict + time.sleep(random.randint(1, 5)) + + attempt_key = f'_{task_name}_ATTEMPT' + success_key = f'_{task_name}_SUCCESS' + + # Check for recent success information + last_success = InvenTreeSetting.get_setting(success_key, '', cache=False) + + if last_success: + try: + last_success = datetime.fromisoformat(last_success) + except ValueError: + last_success = None + + if last_success: + threshold = datetime.now() - timedelta(days=n_days) + + if last_success > threshold: + logger.info(f"Last successful run for '{task_name}' was too recent - skipping task") + return False + + # Check for any information we have about this task + last_attempt = InvenTreeSetting.get_setting(attempt_key, '', cache=False) + + if last_attempt: + try: + last_attempt = datetime.fromisoformat(last_attempt) + except ValueError: + last_attempt = None + + if last_attempt: + # Do not attempt if the most recent *attempt* was within 12 hours + threshold = datetime.now() - timedelta(hours=12) + + if last_attempt > threshold: + logger.info(f"Last attempt for '{task_name}' was too recent - skipping task") + return False + + # Record this attempt + record_task_attempt(task_name) + + # No reason *not* to run this task now + return True + + +def record_task_attempt(task_name: str): + """Record that a multi-day task has been attempted *now*""" + + from common.models import InvenTreeSetting + + logger.info(f"Logging task attempt for '{task_name}'") + + InvenTreeSetting.set_setting(f'_{task_name}_ATTEMPT', datetime.now().isoformat(), None) + + +def record_task_success(task_name: str): + """Record that a multi-day task was successful *now*""" + + from common.models import InvenTreeSetting + + InvenTreeSetting.set_setting(f'_{task_name}_SUCCESS', datetime.now().isoformat(), None) + + def offload_task(taskname, *args, force_async=False, force_sync=False, **kwargs): """Create an AsyncTask if workers are running. This is different to a 'scheduled' task, in that it only runs once! @@ -348,6 +435,14 @@ def check_for_updates(): logger.info("Could not perform 'check_for_updates' - App registry not ready") return + interval = int(common.models.InvenTreeSetting.get_setting('INVENTREE_UPDATE_CHECK_INTERVAL', 7, cache=False)) + + # Check if we should check for updates *today* + if not check_daily_holdoff('check_for_updates', interval): + return + + logger.info("Checking for InvenTree software updates") + headers = {} # If running within github actions, use authentication token @@ -357,7 +452,10 @@ def check_for_updates(): if token: headers['Authorization'] = f"Bearer {token}" - response = requests.get('https://api.github.com/repos/inventree/inventree/releases/latest', headers=headers) + response = requests.get( + 'https://api.github.com/repos/inventree/inventree/releases/latest', + headers=headers + ) if response.status_code != 200: raise ValueError(f'Unexpected status code from GitHub API: {response.status_code}') # pragma: no cover @@ -389,6 +487,9 @@ def check_for_updates(): None ) + # Record that this task was successful + record_task_success('check_for_updates') + @scheduled_task(ScheduledTask.DAILY) def update_exchange_rates(): @@ -435,68 +536,26 @@ def update_exchange_rates(): @scheduled_task(ScheduledTask.DAILY) def run_backup(): """Run the backup command.""" + from common.models import InvenTreeSetting if not InvenTreeSetting.get_setting('INVENTREE_BACKUP_ENABLE', False, cache=False): # Backups are not enabled - exit early return - logger.info("Performing automated database backup task") + interval = int(InvenTreeSetting.get_setting('INVENTREE_BACKUP_DAYS', 1, cache=False)) - # Sleep a random number of seconds to prevent worker conflict - time.sleep(random.randint(1, 5)) - - # Check for records of previous backup attempts - last_attempt = InvenTreeSetting.get_setting('_INVENTREE_BACKUP_ATTEMPT', '', cache=False) - last_success = InvenTreeSetting.get_setting('_INVENTREE_BACKUP_SUCCESS', '', cache=False) - - try: - backup_n_days = int(InvenTreeSetting.get_setting('_INVENTREE_BACKUP_DAYS', 1, cache=False)) - except Exception: - backup_n_days = 1 - - if last_attempt: - try: - last_attempt = datetime.fromisoformat(last_attempt) - except ValueError: - last_attempt = None - - if last_attempt: - # Do not attempt if the 'last attempt' at backup was within 12 hours - threshold = datetime.now() - timedelta(hours=12) - - if last_attempt > threshold: - logger.info('Last backup attempt was too recent - skipping backup operation') - return - - # Record the timestamp of most recent backup attempt - InvenTreeSetting.set_setting('_INVENTREE_BACKUP_ATTEMPT', datetime.now().isoformat(), None) - - if not last_attempt: - # If there is no record of a previous attempt, exit quickly - # This prevents the backup operation from happening when the server first launches, for example - logger.info("No previous backup attempts recorded - waiting until tomorrow") + # Check if should run this task *today* + if not check_daily_holdoff('run_backup', interval): return - if last_success: - try: - last_success = datetime.fromisoformat(last_success) - except ValueError: - last_success = None - - # Exit early if the backup was successful within the number of required days - if last_success: - threshold = datetime.now() - timedelta(days=backup_n_days) - - if last_success > threshold: - logger.info('Last successful backup was too recent - skipping backup operation') - return + logger.info("Performing automated database backup task") call_command("dbbackup", noinput=True, clean=True, compress=True, interactive=False) call_command("mediabackup", noinput=True, clean=True, compress=True, interactive=False) - # Record the timestamp of most recent backup success - InvenTreeSetting.set_setting('_INVENTREE_BACKUP_SUCCESS', datetime.now().isoformat(), None) + # Record that this task was successful + record_task_success('run_backup') def send_email(subject, body, recipients, from_email=None, html_message=None): diff --git a/InvenTree/InvenTree/tests.py b/InvenTree/InvenTree/tests.py index bc90bb063d..2c7f3e726c 100644 --- a/InvenTree/InvenTree/tests.py +++ b/InvenTree/InvenTree/tests.py @@ -3,6 +3,7 @@ import json import os import time +from datetime import datetime, timedelta from decimal import Decimal from unittest import mock @@ -906,6 +907,58 @@ class TestOffloadTask(helpers.InvenTreeTestCase): force_async=True ) + def test_daily_holdoff(self): + """Tests for daily task holdoff helper functions""" + + import InvenTree.tasks + + with self.assertLogs(logger='inventree', level='INFO') as cm: + # With a non-positive interval, task will not run + result = InvenTree.tasks.check_daily_holdoff('some_task', 0) + self.assertFalse(result) + self.assertIn('Specified interval', str(cm.output)) + + with self.assertLogs(logger='inventree', level='INFO') as cm: + # First call should run without issue + result = InvenTree.tasks.check_daily_holdoff('dummy_task') + self.assertTrue(result) + self.assertIn("Logging task attempt for 'dummy_task'", str(cm.output)) + + with self.assertLogs(logger='inventree', level='INFO') as cm: + # An attempt has been logged, but it is too recent + result = InvenTree.tasks.check_daily_holdoff('dummy_task') + self.assertFalse(result) + self.assertIn("Last attempt for 'dummy_task' was too recent", str(cm.output)) + + # Mark last attempt a few days ago - should now return True + t_old = datetime.now() - timedelta(days=3) + t_old = t_old.isoformat() + InvenTreeSetting.set_setting('_dummy_task_ATTEMPT', t_old, None) + + result = InvenTree.tasks.check_daily_holdoff('dummy_task', 5) + self.assertTrue(result) + + # Last attempt should have been updated + self.assertNotEqual(t_old, InvenTreeSetting.get_setting('_dummy_task_ATTEMPT', '', cache=False)) + + # Last attempt should prevent us now + with self.assertLogs(logger='inventree', level='INFO') as cm: + result = InvenTree.tasks.check_daily_holdoff('dummy_task') + self.assertFalse(result) + self.assertIn("Last attempt for 'dummy_task' was too recent", str(cm.output)) + + # Configure so a task was successful too recently + InvenTreeSetting.set_setting('_dummy_task_ATTEMPT', t_old, None) + InvenTreeSetting.set_setting('_dummy_task_SUCCESS', t_old, None) + + with self.assertLogs(logger='inventree', level='INFO') as cm: + result = InvenTree.tasks.check_daily_holdoff('dummy_task', 7) + self.assertFalse(result) + self.assertIn('Last successful run for', str(cm.output)) + + result = InvenTree.tasks.check_daily_holdoff('dummy_task', 2) + self.assertTrue(result) + class BarcodeMixinTest(helpers.InvenTreeTestCase): """Tests for the InvenTreeBarcodeMixin mixin class""" diff --git a/InvenTree/common/models.py b/InvenTree/common/models.py index 7495d0c21c..200ab54845 100644 --- a/InvenTree/common/models.py +++ b/InvenTree/common/models.py @@ -984,6 +984,17 @@ class InvenTreeSetting(BaseInvenTreeSetting): ] }, + 'INVENTREE_UPDATE_CHECK_INTERVAL': { + 'name': _('Update Check Inverval'), + 'description': _('How often to check for updates (set to zero to disable)'), + 'validator': [ + int, + MinValueValidator(0), + ], + 'default': 7, + 'units': _('days'), + }, + 'INVENTREE_BACKUP_ENABLE': { 'name': _('Automatic Backup'), 'description': _('Enable automatic backup of database and media files'), @@ -992,20 +1003,21 @@ class InvenTreeSetting(BaseInvenTreeSetting): }, 'INVENTREE_BACKUP_DAYS': { - 'name': _('Days Between Backup'), + 'name': _('Auto Backup Interval'), 'description': _('Specify number of days between automated backup events'), 'validator': [ int, MinValueValidator(1), ], 'default': 1, + 'units': _('days'), }, 'INVENTREE_DELETE_TASKS_DAYS': { - 'name': _('Delete Old Tasks'), + 'name': _('Task Deletion Interval'), 'description': _('Background task results will be deleted after specified number of days'), 'default': 30, - 'units': 'days', + 'units': _('days'), 'validator': [ int, MinValueValidator(7), @@ -1013,10 +1025,10 @@ class InvenTreeSetting(BaseInvenTreeSetting): }, 'INVENTREE_DELETE_ERRORS_DAYS': { - 'name': _('Delete Error Logs'), + 'name': _('Error Log Deletion Interval'), 'description': _('Error logs will be deleted after specified number of days'), 'default': 30, - 'units': 'days', + 'units': _('days'), 'validator': [ int, MinValueValidator(7) @@ -1024,10 +1036,10 @@ class InvenTreeSetting(BaseInvenTreeSetting): }, 'INVENTREE_DELETE_NOTIFICATIONS_DAYS': { - 'name': _('Delete Notifications'), + 'name': _('Notification Deletion Interval'), 'description': _('User notifications will be deleted after specified number of days'), 'default': 30, - 'units': 'days', + 'units': _('days'), 'validator': [ int, MinValueValidator(7), @@ -1233,7 +1245,7 @@ class InvenTreeSetting(BaseInvenTreeSetting): 'name': _('Stock Item Pricing Age'), 'description': _('Exclude stock items older than this number of days from pricing calculations'), 'default': 0, - 'units': 'days', + 'units': _('days'), 'validator': [ int, MinValueValidator(0), @@ -1255,7 +1267,7 @@ class InvenTreeSetting(BaseInvenTreeSetting): }, 'PRICING_UPDATE_DAYS': { - 'name': _('Pricing Rebuild Time'), + 'name': _('Pricing Rebuild Interval'), 'description': _('Number of days before part pricing is automatically updated'), 'units': _('days'), 'default': 30, @@ -1598,10 +1610,10 @@ class InvenTreeSetting(BaseInvenTreeSetting): }, 'STOCKTAKE_DELETE_REPORT_DAYS': { - 'name': _('Delete Old Reports'), + 'name': _('Report Deletion Interval'), 'description': _('Stocktake reports will be deleted after specified number of days'), 'default': 30, - 'units': 'days', + 'units': _('days'), 'validator': [ int, MinValueValidator(7), diff --git a/InvenTree/templates/InvenTree/settings/global.html b/InvenTree/templates/InvenTree/settings/global.html index ba71fed465..901d0b2a66 100644 --- a/InvenTree/templates/InvenTree/settings/global.html +++ b/InvenTree/templates/InvenTree/settings/global.html @@ -19,6 +19,7 @@ {% include "InvenTree/settings/setting.html" with key="INVENTREE_INSTANCE_TITLE" icon="fa-info-circle" %} {% include "InvenTree/settings/setting.html" with key="INVENTREE_RESTRICT_ABOUT" icon="fa-info-circle" %} <tr><td colspan='5'></td></tr> + {% include "InvenTree/settings/setting.html" with key="INVENTREE_UPDATE_CHECK_INTERVAL" icon="fa-calendar-alt" %} {% include "InvenTree/settings/setting.html" with key="INVENTREE_DOWNLOAD_FROM_URL" icon="fa-cloud-download-alt" %} {% include "InvenTree/settings/setting.html" with key="INVENTREE_DOWNLOAD_IMAGE_MAX_SIZE" icon="fa-server" %} {% include "InvenTree/settings/setting.html" with key="INVENTREE_DOWNLOAD_FROM_URL_USER_AGENT" icon="fa-server" %}