Skip to content

Commit

Permalink
Fix bug, None was used instead of empty for DB outage (ansible#14463)
Browse files Browse the repository at this point in the history
  • Loading branch information
AlanCoding authored and djyasin committed Sep 11, 2024
1 parent 15f6f09 commit da97d63
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 0 deletions.
4 changes: 4 additions & 0 deletions awx/conf/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,10 @@ def _get_local_with_cache(self, name):
"""Get value while accepting the in-memory cache if key is available"""
with _ctit_db_wrapper(trans_safe=True):
return self._get_local(name)
# If the last line did not return, that means we hit a database error
# in that case, we should not have a local cache value
# thus, return empty as a signal to use the default
return empty

def __getattr__(self, name):
value = empty
Expand Down
16 changes: 16 additions & 0 deletions awx/conf/tests/unit/test_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from django.conf import LazySettings
from django.core.cache.backends.locmem import LocMemCache
from django.core.exceptions import ImproperlyConfigured
from django.db.utils import Error as DBError
from django.utils.translation import gettext_lazy as _
import pytest

Expand Down Expand Up @@ -331,3 +332,18 @@ def test_in_memory_cache_works(settings):
with mock.patch.object(settings, '_get_local') as mock_get:
assert settings.AWX_VAR == 'DEFAULT'
mock_get.assert_not_called()


@pytest.mark.defined_in_file(AWX_VAR=[])
def test_getattr_with_database_error(settings):
"""
If a setting is defined via the registry and has a null-ish default which is not None
then referencing that setting during a database outage should give that default
this is regression testing for a bug where it would return None
"""
settings.registry.register('AWX_VAR', field_class=fields.StringListField, default=[], category=_('System'), category_slug='system')
settings._awx_conf_memoizedcache.clear()

with mock.patch('django.db.backends.base.base.BaseDatabaseWrapper.ensure_connection') as mock_ensure:
mock_ensure.side_effect = DBError('for test')
assert settings.AWX_VAR == []

0 comments on commit da97d63

Please sign in to comment.