Add a setting controlling whether cache-machine is enabled or not
- Also, use that setting to disable cache-machine by default in tests, locally and on dev. - Tweak translations tests to not depend on cache-machine at all (remove dependency on ModelBase, depend on django models only + the translation transformer)
This commit is contained in:
Mathieu Pillard
Родитель
a31cd814f7
Коммит
c3e6d3cc1f
|
|
@ -20,6 +20,9 @@ INSTALLED_APPS += (
|
|||
|
||||
FILESYSTEM_CACHE_ROOT = os.path.join(TMP_PATH, 'cache')
|
||||
|
||||
# Disable cache-machine locally and in tests to prepare for its removal.
|
||||
CACHE_MACHINE_ENABLED = False
|
||||
|
||||
# Using locmem deadlocks in certain scenarios. This should all be fixed,
|
||||
# hopefully, in Django1.7. At that point, we may try again, and remove this to
|
||||
# not require memcache installation for newcomers.
|
||||
|
|
|
|||
|
|
@ -951,4 +951,4 @@ class CompatOverrideView(ListAPIView):
|
|||
raise exceptions.ParseError(
|
||||
'Empty, or no, guid parameter provided.')
|
||||
return queryset.filter(guid__in=guids).transform(
|
||||
CompatOverride.transformer)
|
||||
CompatOverride.transformer).order_by('-pk')
|
||||
|
|
|
|||
|
|
@ -23,8 +23,11 @@ from olympia.translations.hold import save_translations
|
|||
from . import search
|
||||
|
||||
|
||||
# Store whether we should be skipping cache-machine for this thread or not.
|
||||
# Note that because the value is initialized at import time, we can't use
|
||||
# override_settings() on CACHE_MACHINE_ENABLED.
|
||||
_locals = threading.local()
|
||||
_locals.skip_cache = False
|
||||
_locals.skip_cache = not settings.CACHE_MACHINE_ENABLED
|
||||
|
||||
log = olympia.core.logger.getLogger('z.addons')
|
||||
|
||||
|
|
@ -43,7 +46,7 @@ def use_master():
|
|||
@contextlib.contextmanager
|
||||
def skip_cache():
|
||||
"""Within this context, no queries come from cache."""
|
||||
old = getattr(_locals, 'skip_cache', False)
|
||||
old = getattr(_locals, 'skip_cache', not settings.CACHE_MACHINE_ENABLED)
|
||||
_locals.skip_cache = True
|
||||
try:
|
||||
yield
|
||||
|
|
|
|||
|
|
@ -1,6 +1,7 @@
|
|||
import mock
|
||||
import pytest
|
||||
|
||||
from django.conf import settings
|
||||
from django.core.files.storage import default_storage as storage
|
||||
|
||||
from mock import Mock
|
||||
|
|
@ -29,14 +30,22 @@ class ManualOrderTest(TestCase):
|
|||
|
||||
|
||||
def test_skip_cache():
|
||||
assert not getattr(amo_models._locals, 'skip_cache', False)
|
||||
assert (
|
||||
getattr(amo_models._locals, 'skip_cache') is
|
||||
not settings.CACHE_MACHINE_ENABLED)
|
||||
|
||||
setattr(amo_models._locals, 'skip_cache', False)
|
||||
|
||||
with amo_models.skip_cache():
|
||||
assert amo_models._locals.skip_cache
|
||||
with amo_models.skip_cache():
|
||||
assert amo_models._locals.skip_cache
|
||||
assert amo_models._locals.skip_cache
|
||||
|
||||
assert not amo_models._locals.skip_cache
|
||||
|
||||
setattr(amo_models._locals, 'skip_cache', settings.CACHE_MACHINE_ENABLED)
|
||||
|
||||
|
||||
def test_use_master():
|
||||
local = amo_models.multidb.pinning._locals
|
||||
|
|
|
|||
|
|
@ -84,6 +84,9 @@ SLAVE_DATABASES = ['slave']
|
|||
|
||||
CACHE_MIDDLEWARE_KEY_PREFIX = CACHE_PREFIX
|
||||
|
||||
# Disable cache-machine on dev to prepare for its removal.
|
||||
CACHE_MACHINE_ENABLED = False
|
||||
|
||||
CACHES = {
|
||||
'filesystem': {
|
||||
'BACKEND': 'django.core.cache.backends.filebased.FileBasedCache',
|
||||
|
|
|
|||
|
|
@ -1006,6 +1006,8 @@ MINIFY_BUNDLES = {
|
|||
|
||||
|
||||
# Caching
|
||||
CACHE_MACHINE_ENABLED = True
|
||||
|
||||
# Prefix for cache keys (will prevent collisions when running parallel copies)
|
||||
CACHE_PREFIX = 'amo:%s:' % build_id
|
||||
KEY_PREFIX = CACHE_PREFIX
|
||||
|
|
|
|||
|
|
@ -789,20 +789,18 @@ class TestRatingViewSetGet(TestCase):
|
|||
assert Rating.unfiltered.count() == 3
|
||||
|
||||
cache.clear()
|
||||
with self.assertNumQueries(7):
|
||||
# 7 queries:
|
||||
# - One for the reviews count
|
||||
# - One for the reviews ids (cache-machine FETCH_BY_ID)
|
||||
# - One for the reviews fields
|
||||
# - One for the reviews translations
|
||||
with self.assertNumQueries(6):
|
||||
# 6 queries:
|
||||
# - One for the ratings count (pagination)
|
||||
# - One for the ratings themselves
|
||||
# - One for the ratings translations
|
||||
# - One for the replies (there aren't any, but we don't know
|
||||
# that without making a query)
|
||||
# - Two for opening and closing a transaction/savepoint
|
||||
# (https://github.com/mozilla/addons-server/issues/3610)
|
||||
#
|
||||
# We patch get_addon_object() to avoid the add-on related queries,
|
||||
# which would pollute the result. In the real world those queries
|
||||
# would often be in the cache.
|
||||
# which would pollute the result.
|
||||
with mock.patch('olympia.ratings.views.RatingViewSet'
|
||||
'.get_addon_object') as get_addon_object:
|
||||
get_addon_object.return_value = self.addon
|
||||
|
|
@ -840,15 +838,13 @@ class TestRatingViewSetGet(TestCase):
|
|||
assert Rating.unfiltered.count() == 5
|
||||
|
||||
cache.clear()
|
||||
with self.assertNumQueries(9):
|
||||
with self.assertNumQueries(7):
|
||||
# 9 queries:
|
||||
# - One for the reviews count
|
||||
# - One for the reviews ids (cache-machine FETCH_BY_ID)
|
||||
# - One for the reviews fields
|
||||
# - One for the reviews translations
|
||||
# - One for the replies ids
|
||||
# - One for the replies fields
|
||||
# - One for the replies translations
|
||||
# - One for the ratings count
|
||||
# - One for the ratings
|
||||
# - One for the ratings translations
|
||||
# - One for the ratings fields
|
||||
# - One for the ratings translations
|
||||
# - Two for opening and closing a transaction/savepoint
|
||||
# (https://github.com/mozilla/addons-server/issues/3610)
|
||||
#
|
||||
|
|
|
|||
|
|
@ -744,7 +744,7 @@ class TestReviewerScore(TestCase):
|
|||
with self.assertNumQueries(0):
|
||||
ReviewerScore.get_recent(self.user)
|
||||
|
||||
with self.assertNumQueries(3):
|
||||
with self.assertNumQueries(2):
|
||||
ReviewerScore.get_leaderboards(self.user)
|
||||
with self.assertNumQueries(0):
|
||||
ReviewerScore.get_leaderboards(self.user)
|
||||
|
|
@ -761,7 +761,7 @@ class TestReviewerScore(TestCase):
|
|||
ReviewerScore.get_total(self.user)
|
||||
with self.assertNumQueries(1):
|
||||
ReviewerScore.get_recent(self.user)
|
||||
with self.assertNumQueries(3):
|
||||
with self.assertNumQueries(2):
|
||||
ReviewerScore.get_leaderboards(self.user)
|
||||
with self.assertNumQueries(1):
|
||||
ReviewerScore.get_breakdown(self.user)
|
||||
|
|
|
|||
|
|
@ -412,10 +412,10 @@ class TranslationTestCase(BaseTestCase):
|
|||
|
||||
obj.description.delete()
|
||||
|
||||
obj = TranslatedModel.objects.no_cache().get(id=1)
|
||||
obj = TranslatedModel.objects.get(id=1)
|
||||
assert obj.description_id is None
|
||||
assert obj.description is None
|
||||
assert not Translation.objects.no_cache().filter(id=trans_id).exists()
|
||||
assert not Translation.objects.filter(id=trans_id).exists()
|
||||
|
||||
@patch.object(TranslatedModel, 'get_fallback', create=True)
|
||||
def test_delete_keep_other_translations(self, get_fallback):
|
||||
|
|
@ -431,9 +431,8 @@ class TranslationTestCase(BaseTestCase):
|
|||
|
||||
obj.name.delete()
|
||||
|
||||
obj = TranslatedModel.objects.no_cache().get(id=1)
|
||||
assert Translation.objects.no_cache().filter(
|
||||
id=orig_name_id).count() == 1
|
||||
obj = TranslatedModel.objects.get(id=1)
|
||||
assert Translation.objects.filter(id=orig_name_id).count() == 1
|
||||
|
||||
# We shouldn't have set name_id to None.
|
||||
assert obj.name_id == orig_name_id
|
||||
|
|
@ -481,7 +480,7 @@ class TranslationMultiDbTests(TransactionTestCase):
|
|||
# Make sure we are in a clean environnement.
|
||||
self.reset_queries()
|
||||
TranslatedModel.objects.get(pk=1)
|
||||
assert len(connections['default'].queries) == 3
|
||||
assert len(connections['default'].queries) == 2
|
||||
|
||||
@override_settings(DEBUG=True)
|
||||
@patch('multidb.get_slave', lambda: 'slave-2')
|
||||
|
|
@ -493,7 +492,7 @@ class TranslationMultiDbTests(TransactionTestCase):
|
|||
TranslatedModel.objects.get(pk=1)
|
||||
assert len(connections['default'].queries) == 0
|
||||
assert len(connections['slave-1'].queries) == 0
|
||||
assert len(connections['slave-2'].queries) == 3
|
||||
assert len(connections['slave-2'].queries) == 2
|
||||
|
||||
@override_settings(DEBUG=True)
|
||||
@patch('multidb.get_slave', lambda: 'slave-2')
|
||||
|
|
@ -505,7 +504,7 @@ class TranslationMultiDbTests(TransactionTestCase):
|
|||
|
||||
TranslatedModel.objects.using('slave-1').get(pk=1)
|
||||
assert len(connections['default'].queries) == 0
|
||||
assert len(connections['slave-1'].queries) == 3
|
||||
assert len(connections['slave-1'].queries) == 2
|
||||
assert len(connections['slave-2'].queries) == 0
|
||||
|
||||
@override_settings(DEBUG=True)
|
||||
|
|
@ -517,7 +516,7 @@ class TranslationMultiDbTests(TransactionTestCase):
|
|||
|
||||
with multidb.pinning.use_master:
|
||||
TranslatedModel.objects.get(pk=1)
|
||||
assert len(connections['default'].queries) == 3
|
||||
assert len(connections['default'].queries) == 2
|
||||
assert len(connections['slave-1'].queries) == 0
|
||||
assert len(connections['slave-2'].queries) == 0
|
||||
|
||||
|
|
|
|||
|
|
@ -1,27 +1,29 @@
|
|||
from django.db import models
|
||||
|
||||
from olympia.amo.models import ModelBase
|
||||
from olympia.amo.models import UncachedManagerBase, UncachedModelBase
|
||||
from olympia.translations.fields import (
|
||||
LinkifiedField, PurifiedField, TranslatedField, save_signal)
|
||||
|
||||
|
||||
class TranslatedModel(ModelBase):
|
||||
class TranslatedModel(UncachedModelBase):
|
||||
name = TranslatedField()
|
||||
description = TranslatedField()
|
||||
default_locale = models.CharField(max_length=10)
|
||||
no_locale = TranslatedField(require_locale=False)
|
||||
|
||||
objects = UncachedManagerBase()
|
||||
|
||||
|
||||
models.signals.pre_save.connect(save_signal, sender=TranslatedModel,
|
||||
dispatch_uid='testapp_translatedmodel')
|
||||
|
||||
|
||||
class UntranslatedModel(ModelBase):
|
||||
class UntranslatedModel(UncachedModelBase):
|
||||
"""Make sure nothing is broken when a model doesn't have translations."""
|
||||
number = models.IntegerField()
|
||||
|
||||
|
||||
class FancyModel(ModelBase):
|
||||
class FancyModel(UncachedModelBase):
|
||||
"""Mix it up with purified and linkified fields."""
|
||||
purified = PurifiedField()
|
||||
linkified = LinkifiedField()
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче