Skip to content

Commit

Permalink
English string validation to error code validation
Browse files Browse the repository at this point in the history
  • Loading branch information
dmzoneill committed Mar 11, 2024
1 parent b076cb0 commit ca8085f
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 24 deletions.
33 changes: 21 additions & 12 deletions awx/conf/settings.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Python
import contextlib
import logging
import psycopg
import threading
import time
import os
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down
32 changes: 25 additions & 7 deletions awx/main/tasks/system.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import json
import logging
import os
import psycopg
from io import StringIO
from contextlib import redirect_stdout
import shutil
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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):
Expand Down
64 changes: 64 additions & 0 deletions awx/main/tests/unit/tasks/test_system.py
Original file line number Diff line number Diff line change
@@ -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()
26 changes: 21 additions & 5 deletions awx/main/utils/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import yaml
import logging
import time
import psycopg
import os
import subprocess
import re
Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit ca8085f

Please sign in to comment.