Browse Source

ref: make options failures an error during test (#54877)

right now these are silently failing and using the default value -- this
makes this explicit that they are accessing the database
anthony sottile 1 year ago
parent
commit
c5c4a00f36

+ 2 - 0
src/sentry/conf/server.py

@@ -2378,6 +2378,8 @@ SENTRY_TEAM_ROLES = (
 # See sentry/options/__init__.py for more information
 SENTRY_OPTIONS: dict[str, Any] = {}
 SENTRY_DEFAULT_OPTIONS: dict[str, Any] = {}
+# Raise an error in dev on failed lookups
+SENTRY_OPTIONS_COMPLAIN_ON_ERRORS = True
 
 # You should not change this setting after your database has been created
 # unless you have altered all schemas first

+ 4 - 1
src/sentry/options/store.py

@@ -6,6 +6,7 @@ from random import random
 from time import time
 from typing import Any, Optional, Set
 
+from django.conf import settings
 from django.db.utils import OperationalError, ProgrammingError
 from django.utils import timezone
 
@@ -187,7 +188,9 @@ class OptionsStore:
         except (self.model.DoesNotExist, ProgrammingError, OperationalError):
             value = None
         except Exception:
-            if not silent:
+            if settings.SENTRY_OPTIONS_COMPLAIN_ON_ERRORS:
+                raise
+            elif not silent:
                 logger.exception("option.failed-lookup", extra={"key": key.name})
             value = None
         else:

+ 1 - 0
src/sentry/testutils/pytest/sentry.py

@@ -189,6 +189,7 @@ def pytest_configure(config):
             "aws-lambda.python.layer-version": "34",
         }
     )
+    settings.SENTRY_OPTIONS_COMPLAIN_ON_ERRORS = True
 
     settings.VALIDATE_SUPERUSER_ACCESS_CATEGORY_AND_REASON = False
     settings.SENTRY_USE_BIG_INTS = True

+ 4 - 0
tests/sentry/options/test_manager.py

@@ -4,6 +4,7 @@ from unittest.mock import patch
 import pytest
 from django.conf import settings
 from django.core.cache.backends.locmem import LocMemCache
+from django.test import override_settings
 
 from sentry.options.manager import (
     DEFAULT_FLAGS,
@@ -257,6 +258,7 @@ class OptionsManagerTest(TestCase):
         with self.settings(SENTRY_OPTIONS={"prioritize_disk": None}):
             assert self.manager.get("prioritize_disk") == "foo"
 
+    @override_settings(SENTRY_OPTIONS_COMPLAIN_ON_ERRORS=False)
     def test_db_unavailable(self):
         with patch.object(self.store.model.objects, "get_queryset", side_effect=RuntimeError()):
             # we can't update options if the db is unavailable
@@ -278,6 +280,7 @@ class OptionsManagerTest(TestCase):
                     assert self.manager.get("foo") == ""
                     self.store.flush_local_cache()
 
+    @override_settings(SENTRY_OPTIONS_COMPLAIN_ON_ERRORS=False)
     def test_db_and_cache_unavailable(self):
         self.store.cache.clear()
         self.manager.set("foo", "bar")
@@ -293,6 +296,7 @@ class OptionsManagerTest(TestCase):
                         assert self.manager.get("foo") == "baz"
                         self.store.flush_local_cache()
 
+    @override_settings(SENTRY_OPTIONS_COMPLAIN_ON_ERRORS=False)
     def test_cache_unavailable(self):
         self.manager.set("foo", "bar")
         self.store.flush_local_cache()

+ 4 - 0
tests/sentry/options/test_store.py

@@ -5,6 +5,7 @@ from uuid import uuid1
 import pytest
 from django.conf import settings
 from django.core.cache.backends.locmem import LocMemCache
+from django.test import override_settings
 
 from sentry.models import Option
 from sentry.options.manager import OptionsManager, UpdateChannel
@@ -60,6 +61,7 @@ class OptionsStoreTest(TestCase):
         with pytest.raises(AssertionError):
             store.delete(key)
 
+    @override_settings(SENTRY_OPTIONS_COMPLAIN_ON_ERRORS=False)
     def test_db_and_cache_unavailable(self):
         store, key = self.store, self.key
         with patch.object(Option.objects, "get_queryset", side_effect=RuntimeError()):
@@ -80,6 +82,7 @@ class OptionsStoreTest(TestCase):
                 store.flush_local_cache()
                 assert store.get(key) is None
 
+    @override_settings(SENTRY_OPTIONS_COMPLAIN_ON_ERRORS=False)
     @patch("sentry.options.store.time")
     def test_key_with_grace(self, mocked_time):
         store, key = self.store, self.make_key(10, 10)
@@ -99,6 +102,7 @@ class OptionsStoreTest(TestCase):
                 # It should have also been evicted
                 assert not store._local_cache
 
+    @override_settings(SENTRY_OPTIONS_COMPLAIN_ON_ERRORS=False)
     @patch("sentry.options.store.time")
     def test_key_ttl(self, mocked_time):
         store, key = self.store, self.make_key(10, 0)