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

@@ -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
 
+    interval = int(InvenTreeSetting.get_setting('INVENTREE_BACKUP_DAYS', 1, cache=False))
+
+    # Check if should run this task *today*
+    if not check_daily_holdoff('run_backup', interval):
+        return
+
     logger.info("Performing automated database backup task")
 
-    # 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")
-        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
-
     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
+    # Record that this task was successful
-    InvenTreeSetting.set_setting('_INVENTREE_BACKUP_SUCCESS', datetime.now().isoformat(), None)
+    record_task_success('run_backup')
 
 
 def send_email(subject, body, recipients, from_email=None, html_message=None):

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"""

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),

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" %}