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 authored and David O Neill committed Mar 7, 2024
1 parent 727278a commit f019fc4
Show file tree
Hide file tree
Showing 5 changed files with 139 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
11 changes: 10 additions & 1 deletion awx/main/models/inventory.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,7 @@ def update_computed_fields(self):
"""
Update model fields that are computed from database relationships.
"""
rowcount = 0 # update,add,delete sql query should have affected rows in the sql resultset rowcount.
logger.debug("Going to update inventory computed fields, pk={0}".format(self.pk))
start_time = time.time()
active_hosts = self.hosts
Expand Down Expand Up @@ -424,8 +425,16 @@ def update_computed_fields(self):
tasks = self.jobs.filter(status="pending")
for t in tasks:
t.task_impact = t._get_task_impact()
UnifiedJob.objects.bulk_update(tasks, ['task_impact'])
# (method) def bulk_update(
# objs: Iterable[UnifiedJob],
# fields: Sequence[str],
# batch_size: int | None = ...
# ) -> int
# https://pypi.org/project/django-pg-bulk-update/
# Function returns number of updated records.
rowcount = UnifiedJob.objects.bulk_update(tasks, ['task_impact'])
logger.debug("Finished updating inventory computed fields, pk={0}, in {1:.3f} seconds".format(self.pk, time.time() - start_time))
return rowcount

def websocket_emit_status(self, status):
connection.on_commit(
Expand Down
29 changes: 22 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 @@ -786,11 +795,17 @@ def update_inventory_computed_fields(inventory_id):
return
i = i[0]
try:
i.update_computed_fields()
if i.update_computed_fields() == 0:
logger.debug('Duplicate update_inventory_computed_fields task.')
except DatabaseError as e:
if 'did not affect any rows' in str(e):
logger.debug('Exiting duplicate update_inventory_computed_fields task.')
return
# update_computed_fields should never throw an error
# however if it does then lets manage it
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


Expand Down
66 changes: 66 additions & 0 deletions awx/main/tests/unit/tasks/test_system.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
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(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 to raise a DatabaseError
with patch("awx.main.tasks.system.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")

# Ensuring that the function does raise DatabaseError
with pytest.raises(DatabaseError):
update_inventory_computed_fields(1)

# Assertions
mock_filter.assert_called_once_with(id=1)
mock_inventory.update_computed_fields.assert_called_once()
mock_logger.error.assert_called_once()
24 changes: 20 additions & 4 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,10 +1156,25 @@ 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:
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


Expand Down

0 comments on commit f019fc4

Please sign in to comment.