From 4e6fd591809fbde77117eb4226ae462bf082d87a Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Thu, 24 May 2018 13:27:20 -0400 Subject: [PATCH] Handle broken transactions in DB settings getattr This expands the role of the log database error context manager and will actually make itself an exception to the standard ORM behavior of raising an error when any queries are executed inside of a broken transaction. In this particular case it is less risky to continue on with a database query and push the data to memcache than it would be to use default settings values in violation of user's intent. (hopefully) --- awx/conf/settings.py | 76 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 59 insertions(+), 17 deletions(-) diff --git a/awx/conf/settings.py b/awx/conf/settings.py index 2f7970ec2b..986e09037d 100644 --- a/awx/conf/settings.py +++ b/awx/conf/settings.py @@ -15,7 +15,7 @@ from django.conf import LazySettings from django.conf import settings, UserSettingsHolder from django.core.cache import cache as django_cache from django.core.exceptions import ImproperlyConfigured -from django.db import ProgrammingError, OperationalError +from django.db import ProgrammingError, OperationalError, transaction, connection from django.utils.functional import cached_property # Django REST Framework @@ -61,24 +61,66 @@ __all__ = ['SettingsWrapper', 'get_settings_to_cache', 'SETTING_CACHE_NOTSET'] @contextlib.contextmanager -def _log_database_error(): +def _ctit_db_wrapper(trans_safe=False): + ''' + Wrapper to avoid undesired actions by Django ORM when managing settings + if only getting a setting, can use trans_safe=True, which will avoid + throwing errors if the prior context was a broken transaction. + Any database errors will be logged, but exception will be suppressed. + ''' + rollback_set = None + is_atomic = None try: + if trans_safe: + is_atomic = connection.in_atomic_block + if is_atomic: + rollback_set = transaction.get_rollback() + if rollback_set: + logger.debug('Obtaining database settings in spite of broken transaction.') + transaction.set_rollback(False) yield except (ProgrammingError, OperationalError): if 'migrate' in sys.argv and get_tower_migration_version() < '310': logger.info('Using default settings until version 3.1 migration.') else: - # Somewhat ugly - craming the full stack trace into the log message - # the available exc_info does not give information about the real caller - # TODO: replace in favor of stack_info kwarg in python 3 - sio = StringIO.StringIO() - traceback.print_stack(file=sio) - sinfo = sio.getvalue() - sio.close() - sinfo = sinfo.strip('\n') - logger.warning('Database settings are not available, using defaults, logged from:\n{}'.format(sinfo)) + # We want the _full_ traceback with the context + # First we get the current call stack, which constitutes the "top", + # it has the context up to the point where the context manager is used + top_stack = StringIO.StringIO() + traceback.print_stack(file=top_stack) + top_lines = top_stack.getvalue().strip('\n').split('\n') + top_stack.close() + # Get "bottom" stack from the local error that happened + # inside of the "with" block this wraps + exc_type, exc_value, exc_traceback = sys.exc_info() + bottom_stack = StringIO.StringIO() + traceback.print_tb(exc_traceback, file=bottom_stack) + bottom_lines = bottom_stack.getvalue().strip('\n').split('\n') + # Glue together top and bottom where overlap is found + bottom_cutoff = 0 + for i, line in enumerate(bottom_lines): + if line in top_lines: + # start of overlapping section, take overlap from bottom + top_lines = top_lines[:top_lines.index(line)] + bottom_cutoff = i + break + bottom_lines = bottom_lines[bottom_cutoff:] + tb_lines = top_lines + bottom_lines + + tb_string = '\n'.join( + ['Traceback (most recent call last):'] + + tb_lines + + ['{}: {}'.format(exc_type.__name__, str(exc_value))] + ) + bottom_stack.close() + # Log the combined stack + if trans_safe: + logger.warning('Database settings are not available, using defaults, error:\n{}'.format(tb_string)) + else: + logger.error('Error modifying something related to database settings.\n{}'.format(tb_string)) finally: - pass + if trans_safe and is_atomic and rollback_set: + transaction.set_rollback(rollback_set) def filter_sensitive(registry, key, value): @@ -398,7 +440,7 @@ class SettingsWrapper(UserSettingsHolder): def __getattr__(self, name): value = empty if name in self.all_supported_settings: - with _log_database_error(): + with _ctit_db_wrapper(trans_safe=True): value = self._get_local(name) if value is not empty: return value @@ -430,7 +472,7 @@ class SettingsWrapper(UserSettingsHolder): def __setattr__(self, name, value): if name in self.all_supported_settings: - with _log_database_error(): + with _ctit_db_wrapper(): self._set_local(name, value) else: setattr(self.default_settings, name, value) @@ -446,14 +488,14 @@ class SettingsWrapper(UserSettingsHolder): def __delattr__(self, name): if name in self.all_supported_settings: - with _log_database_error(): + with _ctit_db_wrapper(): self._del_local(name) else: delattr(self.default_settings, name) def __dir__(self): keys = [] - with _log_database_error(): + with _ctit_db_wrapper(trans_safe=True): for setting in Setting.objects.filter( key__in=self.all_supported_settings, user__isnull=True): # Skip returning settings that have been overridden but are @@ -470,7 +512,7 @@ class SettingsWrapper(UserSettingsHolder): def is_overridden(self, setting): set_locally = False if setting in self.all_supported_settings: - with _log_database_error(): + with _ctit_db_wrapper(trans_safe=True): set_locally = Setting.objects.filter(key=setting, user__isnull=True).exists() set_on_default = getattr(self.default_settings, 'is_overridden', lambda s: False)(setting) return (set_locally or set_on_default)