caches: Remove unnecessary "in-memory" cache.

This cache was added in da33b72848 to serve as a replacement for the
durable database cache, in development; the previous commit has
switched that to be the non-durable memcached backend.

The special-case for "in-memory" in development is mostly-unnecessary
in contrast to memcached -- `./tools/run-dev.py` flushes memcached on
every startup.  This differs in behaviour slightly, in that if the
codepath is changed and `run-dev` restarts Django, the cache is not
cleared.  This seems an unlikely occurrence, however, and the code
cleanup from its removal is worth it.
This commit is contained in:
Alex Vandiver 2022-04-13 21:53:08 -07:00 committed by Tim Abbott
parent 04ca2e92f7
commit 56058f3316
3 changed files with 9 additions and 18 deletions

View File

@ -15,8 +15,6 @@ from zerver.lib.url_preview.oembed import get_oembed_data
from zerver.lib.url_preview.parsers import GenericParser, OpenGraphParser from zerver.lib.url_preview.parsers import GenericParser, OpenGraphParser
from zerver.lib.url_preview.types import UrlEmbedData, UrlOEmbedData from zerver.lib.url_preview.types import UrlEmbedData, UrlOEmbedData
# Use an in-memory cache for development, to make it easy to develop this code
CACHE_NAME = "default" if not settings.DEVELOPMENT else "in-memory"
# Based on django.core.validators.URLValidator, with ftp support removed. # Based on django.core.validators.URLValidator, with ftp support removed.
link_regex = re.compile( link_regex = re.compile(
r"^(?:http)s?://" # http:// or https:// r"^(?:http)s?://" # http:// or https://
@ -83,7 +81,7 @@ def catch_network_errors(func: Callable[..., Any]) -> Callable[..., Any]:
@catch_network_errors @catch_network_errors
@cache_with_key(preview_url_cache_key, cache_name=CACHE_NAME, with_statsd_key="urlpreview_data") @cache_with_key(preview_url_cache_key, with_statsd_key="urlpreview_data")
def get_link_embed_data( def get_link_embed_data(
url: str, maxwidth: int = 640, maxheight: int = 480 url: str, maxwidth: int = 640, maxheight: int = 480
) -> Optional[UrlEmbedData]: ) -> Optional[UrlEmbedData]:

View File

@ -31,10 +31,6 @@ TEST_CACHES = {
"BACKEND": "django.core.cache.backends.locmem.LocMemCache", "BACKEND": "django.core.cache.backends.locmem.LocMemCache",
"LOCATION": "url-preview", "LOCATION": "url-preview",
}, },
"in-memory": {
"BACKEND": "django.core.cache.backends.locmem.LocMemCache",
"LOCATION": "url-preview",
},
} }
@ -653,7 +649,7 @@ class PreviewTestCase(ZulipTestCase):
with self.settings(TEST_SUITE=False, CACHES=TEST_CACHES): with self.settings(TEST_SUITE=False, CACHES=TEST_CACHES):
with self.assertLogs(level="INFO") as info_logs: with self.assertLogs(level="INFO") as info_logs:
FetchLinksEmbedData().consume(event) FetchLinksEmbedData().consume(event)
cached_data = cache_get(preview_url_cache_key(url), cache_name="in-memory")[0] cached_data = cache_get(preview_url_cache_key(url))[0]
self.assertTrue( self.assertTrue(
"INFO:root:Time spent on get_link_embed_data for http://test.org/audio.mp3: " "INFO:root:Time spent on get_link_embed_data for http://test.org/audio.mp3: "
in info_logs.output[0] in info_logs.output[0]
@ -687,7 +683,7 @@ class PreviewTestCase(ZulipTestCase):
with self.settings(TEST_SUITE=False, CACHES=TEST_CACHES): with self.settings(TEST_SUITE=False, CACHES=TEST_CACHES):
with self.assertLogs(level="INFO") as info_logs: with self.assertLogs(level="INFO") as info_logs:
FetchLinksEmbedData().consume(event) FetchLinksEmbedData().consume(event)
cached_data = cache_get(preview_url_cache_key(url), cache_name="in-memory")[0] cached_data = cache_get(preview_url_cache_key(url))[0]
self.assertTrue( self.assertTrue(
"INFO:root:Time spent on get_link_embed_data for http://test.org/foo.html: " "INFO:root:Time spent on get_link_embed_data for http://test.org/foo.html: "
in info_logs.output[0] in info_logs.output[0]
@ -726,7 +722,7 @@ class PreviewTestCase(ZulipTestCase):
with self.settings(TEST_SUITE=False, CACHES=TEST_CACHES): with self.settings(TEST_SUITE=False, CACHES=TEST_CACHES):
with self.assertLogs(level="INFO") as info_logs: with self.assertLogs(level="INFO") as info_logs:
FetchLinksEmbedData().consume(event) FetchLinksEmbedData().consume(event)
cached_data = cache_get(preview_url_cache_key(url), cache_name="in-memory")[0] cached_data = cache_get(preview_url_cache_key(url))[0]
self.assertTrue( self.assertTrue(
"INFO:root:Time spent on get_link_embed_data for http://test.org/foo.html: " "INFO:root:Time spent on get_link_embed_data for http://test.org/foo.html: "
in info_logs.output[0] in info_logs.output[0]
@ -763,7 +759,7 @@ class PreviewTestCase(ZulipTestCase):
with self.settings(TEST_SUITE=False, CACHES=TEST_CACHES): with self.settings(TEST_SUITE=False, CACHES=TEST_CACHES):
with self.assertLogs(level="INFO") as info_logs: with self.assertLogs(level="INFO") as info_logs:
FetchLinksEmbedData().consume(event) FetchLinksEmbedData().consume(event)
cached_data = cache_get(preview_url_cache_key(url), cache_name="in-memory")[0] cached_data = cache_get(preview_url_cache_key(url))[0]
self.assertTrue( self.assertTrue(
"INFO:root:Time spent on get_link_embed_data for http://test.org/foo.html: " "INFO:root:Time spent on get_link_embed_data for http://test.org/foo.html: "
in info_logs.output[0] in info_logs.output[0]
@ -796,7 +792,7 @@ class PreviewTestCase(ZulipTestCase):
with self.settings(TEST_SUITE=False, CACHES=TEST_CACHES): with self.settings(TEST_SUITE=False, CACHES=TEST_CACHES):
with self.assertLogs(level="INFO") as info_logs: with self.assertLogs(level="INFO") as info_logs:
FetchLinksEmbedData().consume(event) FetchLinksEmbedData().consume(event)
cached_data = cache_get(preview_url_cache_key(url), cache_name="in-memory")[0] cached_data = cache_get(preview_url_cache_key(url))[0]
self.assertTrue( self.assertTrue(
"INFO:root:Time spent on get_link_embed_data for http://test.org/: " "INFO:root:Time spent on get_link_embed_data for http://test.org/: "
in info_logs.output[0] in info_logs.output[0]
@ -844,7 +840,7 @@ class PreviewTestCase(ZulipTestCase):
) )
# This did not get cached -- hence the lack of [0] on the cache_get # This did not get cached -- hence the lack of [0] on the cache_get
cached_data = cache_get(preview_url_cache_key(url), cache_name="in-memory") cached_data = cache_get(preview_url_cache_key(url))
self.assertIsNone(cached_data) self.assertIsNone(cached_data)
msg.refresh_from_db() msg.refresh_from_db()
@ -881,7 +877,7 @@ class PreviewTestCase(ZulipTestCase):
) )
# FIXME: Should we really cache this, especially without cache invalidation? # FIXME: Should we really cache this, especially without cache invalidation?
cached_data = cache_get(preview_url_cache_key(error_url), cache_name="in-memory")[0] cached_data = cache_get(preview_url_cache_key(error_url))[0]
self.assertIsNone(cached_data) self.assertIsNone(cached_data)
msg.refresh_from_db() msg.refresh_from_db()
@ -921,7 +917,7 @@ class PreviewTestCase(ZulipTestCase):
lambda *args, **kwargs: mocked_data, lambda *args, **kwargs: mocked_data,
): ):
FetchLinksEmbedData().consume(event) FetchLinksEmbedData().consume(event)
cached_data = cache_get(preview_url_cache_key(url), cache_name="in-memory")[0] cached_data = cache_get(preview_url_cache_key(url))[0]
self.assertTrue( self.assertTrue(
"INFO:root:Time spent on get_link_embed_data for http://test.org/: " "INFO:root:Time spent on get_link_embed_data for http://test.org/: "
in info_logs.output[0] in info_logs.output[0]

View File

@ -361,9 +361,6 @@ CACHES = {
"CULL_FREQUENCY": 10, "CULL_FREQUENCY": 10,
}, },
}, },
"in-memory": {
"BACKEND": "django.core.cache.backends.locmem.LocMemCache",
},
} }
######################################################################## ########################################################################