Don't use English for string comparisons

awx/conf/settings.py

@@ -1,6 +1,7 @@
 # Python
 import contextlib
 import logging
+import psycopg
 import threading
 import time
 import os
@@ -13,7 +14,7 @@
 from django.core.cache import cache as django_cache
 from django.core.exceptions import ImproperlyConfigured, SynchronousOnlyOperation
 from django.db import transaction, connection
-from django.db.utils import Error as DBError, ProgrammingError
+from django.db.utils import DatabaseError, ProgrammingError
 from django.utils.functional import cached_property
 
 # Django REST Framework
@@ -80,18 +81,26 @@ def _ctit_db_wrapper(trans_safe=False):
                     logger.debug('Obtaining database settings in spite of broken transaction.')
                     transaction.set_rollback(False)
         yield
-    except DBError as exc:
+    except ProgrammingError as e:
+        # Exception raised for programming errors
+        # Examples may be table not found or already exists,
+        # this generally means we can't fetch Tower configuration
+        # because the database hasn't actually finished migrating yet;
+        # this is usually a sign that a service in a container (such as ws_broadcast)
+        # has come up *before* the database has finished migrating, and
+        # especially that the conf.settings table doesn't exist yet
+        # syntax error in the SQL statement, wrong number of parameters specified, etc.
         if trans_safe:
-            level = logger.warning
-            if isinstance(exc, ProgrammingError):
-                if 'relation' in str(exc) and 'does not exist' in str(exc):
-                    # this generally means we can't fetch Tower configuration
-                    # because the database hasn't actually finished migrating yet;
-                    # this is usually a sign that a service in a container (such as ws_broadcast)
-                    # has come up *before* the database has finished migrating, and
-                    # especially that the conf.settings table doesn't exist yet
-                    level = logger.debug
-            level(f'Database settings are not available, using defaults. error: {str(exc)}')
+            logger.debug(f'Database settings are not available, using defaults. error: {str(e)}')
+        else:
+            logger.exception('Error modifying something related to database settings.')
+    except DatabaseError as e:
+        if trans_safe:
+            cause = e.__cause__
+            if cause and hasattr(cause, 'sqlstate'):
+                sqlstate = cause.sqlstate
+                sqlstate_str = psycopg.errors.lookup(sqlstate)
+                logger.error('SQL Error state: {} - {}'.format(sqlstate, sqlstate_str))
         else:
             logger.exception('Error modifying something related to database settings.')
     finally:

awx/main/tasks/system.py

@@ -6,6 +6,7 @@
 import json
 import logging
 import os
+import psycopg
 from io import StringIO
 from contextlib import redirect_stdout
 import shutil
@@ -630,10 +631,18 @@ def cluster_node_heartbeat(dispatch_time=None, worker_tasks=None):
                 logger.error("Host {} last checked in at {}, marked as lost.".format(other_inst.hostname, other_inst.last_seen))
 
         except DatabaseError as e:
-            if 'did not affect any rows' in str(e):
-                logger.debug('Another instance has marked {} as lost'.format(other_inst.hostname))
+            cause = e.__cause__
+            if cause and hasattr(cause, 'sqlstate'):
+                sqlstate = cause.sqlstate
+                sqlstate_str = psycopg.errors.lookup(sqlstate)
+                logger.debug('SQL Error state: {} - {}'.format(sqlstate, sqlstate_str))
+
+                if sqlstate == psycopg.errors.NoData:
+                    logger.debug('Another instance has marked {} as lost'.format(other_inst.hostname))
+                else:
+                    logger.exception("Error marking {} as lost.".format(other_inst.hostname))
             else:
-                logger.exception('Error marking {} as lost'.format(other_inst.hostname))
+                logger.exception('No SQL state available.  Error marking {} as lost'.format(other_inst.hostname))
 
     # Run local reaper
     if worker_tasks is not None:
@@ -788,10 +797,19 @@ def update_inventory_computed_fields(inventory_id):
     try:
         i.update_computed_fields()
     except DatabaseError as e:
-        if 'did not affect any rows' in str(e):
-            logger.debug('Exiting duplicate update_inventory_computed_fields task.')
-            return
-        raise
+        # https://github.com/django/django/blob/eff21d8e7a1cb297aedf1c702668b590a1b618f3/django/db/models/base.py#L1105
+        # django raises DatabaseError("Forced update did not affect any rows.")
+
+        # if sqlstate is set then there was a database error and otherwise will re-raise that error
+        cause = e.__cause__
+        if cause and hasattr(cause, 'sqlstate'):
+            sqlstate = cause.sqlstate
+            sqlstate_str = psycopg.errors.lookup(sqlstate)
+            logger.error('SQL Error state: {} - {}'.format(sqlstate, sqlstate_str))
+            raise
+
+        # otherwise
+        logger.debug('Exiting duplicate update_inventory_computed_fields task.')
 
 
 def update_smart_memberships_for_inventory(smart_inventory):

awx/main/tests/unit/tasks/test_system.py

@@ -0,0 +1,64 @@
+import pytest
+from unittest.mock import MagicMock, patch
+from awx.main.tasks.system import update_inventory_computed_fields
+from awx.main.models import Inventory
+from django.db import DatabaseError
+
+
+@pytest.fixture
+def mock_logger():
+    with patch("awx.main.tasks.system.logger") as logger:
+        yield logger
+
+
+@pytest.fixture
+def mock_inventory():
+    return MagicMock(spec=Inventory)
+
+
+def test_update_inventory_computed_fields_existing_inventory(mock_logger, mock_inventory):
+    # Mocking the Inventory.objects.filter method to return a non-empty queryset
+    with patch("awx.main.tasks.system.Inventory.objects.filter") as mock_filter:
+        mock_filter.return_value.exists.return_value = True
+        mock_filter.return_value.__getitem__.return_value = mock_inventory
+
+        # Mocking the update_computed_fields method
+        with patch.object(mock_inventory, "update_computed_fields") as mock_update_computed_fields:
+            update_inventory_computed_fields(1)
+
+            # Assertions
+            mock_filter.assert_called_once_with(id=1)
+            mock_update_computed_fields.assert_called_once()
+
+            # You can add more assertions based on your specific requirements
+
+
+def test_update_inventory_computed_fields_missing_inventory(mock_logger):
+    # Mocking the Inventory.objects.filter method to return an empty queryset
+    with patch("awx.main.tasks.system.Inventory.objects.filter") as mock_filter:
+        mock_filter.return_value.exists.return_value = False
+
+        update_inventory_computed_fields(1)
+
+        # Assertions
+        mock_filter.assert_called_once_with(id=1)
+        mock_logger.error.assert_called_once_with("Update Inventory Computed Fields failed due to missing inventory: 1")
+
+
+def test_update_inventory_computed_fields_database_error_nosqlstate(mock_logger, mock_inventory):
+    # Mocking the Inventory.objects.filter method to return a non-empty queryset
+    with patch("awx.main.tasks.system.Inventory.objects.filter") as mock_filter:
+        mock_filter.return_value.exists.return_value = True
+        mock_filter.return_value.__getitem__.return_value = mock_inventory
+
+        # Mocking the update_computed_fields method
+        with patch.object(mock_inventory, "update_computed_fields") as mock_update_computed_fields:
+            # Simulating the update_computed_fields method to explicitly raise a DatabaseError
+            mock_update_computed_fields.side_effect = DatabaseError("Some error")
+
+            update_inventory_computed_fields(1)
+
+            # Assertions
+            mock_filter.assert_called_once_with(id=1)
+            mock_update_computed_fields.assert_called_once()
+            mock_inventory.update_computed_fields.assert_called_once()

awx/main/utils/common.py

@@ -7,6 +7,7 @@
 import yaml
 import logging
 import time
+import psycopg
 import os
 import subprocess
 import re
@@ -23,7 +24,7 @@
 from django.utils.dateparse import parse_datetime
 from django.utils.translation import gettext_lazy as _
 from django.utils.functional import cached_property
-from django.db import connection, transaction, ProgrammingError, IntegrityError
+from django.db import connection, DatabaseError, transaction, ProgrammingError, IntegrityError
 from django.db.models.fields.related import ForeignObjectRel, ManyToManyField
 from django.db.models.fields.related_descriptors import ForwardManyToOneDescriptor, ManyToManyDescriptor
 from django.db.models.query import QuerySet
@@ -1155,11 +1156,26 @@ def create_partition(tblname, start=None):
                     f'ALTER TABLE {tblname} ATTACH PARTITION {tblname}_{partition_label} '
                     f'FOR VALUES FROM (\'{start_timestamp}\') TO (\'{end_timestamp}\');'
                 )
+
     except (ProgrammingError, IntegrityError) as e:
-        if 'already exists' in str(e):
-            logger.info(f'Caught known error due to partition creation race: {e}')
-        else:
-            raise
+        cause = e.__cause__
+        if cause and hasattr(cause, 'sqlstate'):
+            # 42P07 = DuplicateTable
+            sqlstate = cause.sqlstate
+            sqlstate_str = psycopg.errors.lookup(sqlstate)
+
+            if psycopg.errors.DuplicateTable == sqlstate:
+                logger.info(f'Caught known error due to partition creation race: {e}')
+            else:
+                logger.error('SQL Error state: {} - {}'.format(sqlstate, sqlstate_str))
+                raise
+    except DatabaseError as e:
+        cause = e.__cause__
+        if cause and hasattr(cause, 'sqlstate'):
+            sqlstate = cause.sqlstate
+            sqlstate_str = psycopg.errors.lookup(sqlstate)
+            logger.error('SQL Error state: {} - {}'.format(sqlstate, sqlstate_str))
+        raise
 
 
 def cleanup_new_process(func):