Browse Source

ref: remove python-memcache (#63375)

we're now using the pymemcache backend everywhere!
<!-- Describe your PR here. -->
anthony sottile 1 year ago
parent
commit
018d5e9fe0

+ 0 - 2
requirements-base.txt

@@ -47,8 +47,6 @@ PyJWT>=2.4.0
 pydantic>=1.10.9
 python-dateutil>=2.8.2
 pymemcache
-# TODO: once pymemcache is validated, remove python-memcached
-python-memcached>=1.59
 python-u2flib-server>=5.0.0
 fido2>=0.9.2
 python3-saml>=1.15.0

+ 0 - 1
requirements-dev-frozen.txt

@@ -149,7 +149,6 @@ pytest-rerunfailures==11.0
 pytest-sentry==0.1.11
 pytest-xdist==3.0.2
 python-dateutil==2.8.2
-python-memcached==1.59
 python-rapidjson==1.8
 python-u2flib-server==5.0.0
 python-utils==3.3.3

+ 0 - 1
requirements-frozen.txt

@@ -97,7 +97,6 @@ pyjwt==2.4.0
 pymemcache==4.0.0
 pyparsing==3.0.9
 python-dateutil==2.8.2
-python-memcached==1.59
 python-rapidjson==1.8
 python-u2flib-server==5.0.0
 python-utils==3.3.3

+ 1 - 1
self-hosted/sentry.conf.py

@@ -115,7 +115,7 @@ if memcached:
     memcached_port = env("SENTRY_MEMCACHED_PORT") or "11211"
     CACHES = {
         "default": {
-            "BACKEND": "sentry.utils.memcached.MemcachedCache",
+            "BACKEND": "django.core.cache.backends.memcached.PyMemcacheCache",
             "LOCATION": [memcached + ":" + memcached_port],
             "TIMEOUT": 3600,
         }

+ 1 - 1
src/sentry/data/config/sentry.conf.py.default

@@ -46,7 +46,7 @@ DEBUG = %(debug_flag)s
 #
 # CACHES = {
 #     'default': {
-#         'BACKEND': 'sentry.utils.memcached.MemcachedCache',
+#         'BACKEND': 'django.core.cache.backends.memcached.PyMemcacheCache',
 #         'LOCATION': ['127.0.0.1:11211'],
 #     }
 # }

+ 0 - 11
src/sentry/services/http.py

@@ -48,7 +48,6 @@ class SentryHTTPServer(Service):
         port: int | None = None,
         debug: bool = False,
         workers: int | None = None,
-        validate: bool = True,
         extra_options: dict[str, Any] | None = None,
     ) -> None:
         from django.conf import settings
@@ -56,9 +55,6 @@ class SentryHTTPServer(Service):
         from sentry import options as sentry_options
         from sentry.logging import LoggingFormat
 
-        if validate:
-            self.validate_settings()
-
         host = host or settings.SENTRY_WEB_HOST
         port = port or settings.SENTRY_WEB_PORT
 
@@ -146,13 +142,6 @@ class SentryHTTPServer(Service):
         self.options = options
         self.debug = debug
 
-    def validate_settings(self) -> None:
-        from django.conf import settings as django_settings
-
-        from sentry.utils.settings import validate_settings
-
-        validate_settings(django_settings)
-
     def prepare_environment(self, env: MutableMapping[str, str] | None = None) -> None:
         from django.conf import settings
 

+ 0 - 69
src/sentry/utils/memcached.py

@@ -1,69 +0,0 @@
-"""copied from django 4.0 before removal
-
-modified to fix bug with monkeypatching and remove warnings
-
----
-
-Copyright (c) Django Software Foundation and individual contributors.
-All rights reserved.
-
-Redistribution and use in source and binary forms, with or without modification,
-are permitted provided that the following conditions are met:
-
-    1. Redistributions of source code must retain the above copyright notice,
-       this list of conditions and the following disclaimer.
-
-    2. Redistributions in binary form must reproduce the above copyright
-       notice, this list of conditions and the following disclaimer in the
-       documentation and/or other materials provided with the distribution.
-
-    3. Neither the name of Django nor the names of its contributors may be used
-       to endorse or promote products derived from this software without
-       specific prior written permission.
-
-THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
-ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
-WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
-DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
-ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
-(INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
-LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
-ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-(INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
-SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-"""
-import pickle
-
-from django.core.cache.backends.memcached import BaseMemcachedCache
-
-
-class MemcachedCache(BaseMemcachedCache):
-    "An implementation of a cache binding using python-memcached"
-
-    # python-memcached doesn't support default values in get().
-    # https://github.com/linsomniac/python-memcached/issues/159
-    _missing_key = None
-
-    def __init__(self, server, params):
-        # python-memcached ≥ 1.45 returns None for a nonexistent key in
-        # incr/decr(), python-memcached < 1.45 raises ValueError.
-        import memcache
-
-        super().__init__(server, params, library=memcache, value_not_found_exception=ValueError)
-        self._options = {"pickleProtocol": pickle.HIGHEST_PROTOCOL, **self._options}  # type: ignore[has-type]
-
-    def get(self, key, default=None, version=None):
-        key = self.make_key(key, version=version)
-        self.validate_key(key)
-        val = self._cache.get(key)  # type: ignore[attr-defined]
-        # python-memcached doesn't support default values in get().
-        # https://github.com/linsomniac/python-memcached/issues/159
-        # Remove this method if that issue is fixed.
-        if val is None:
-            return default
-        return val
-
-    def delete(self, key, version=None):
-        key = self.make_key(key, version=version)
-        self.validate_key(key)
-        return bool(self._cache.delete(key))  # type: ignore[attr-defined]

+ 0 - 46
src/sentry/utils/settings.py

@@ -1,49 +1,3 @@
-from sentry.utils.imports import import_string
-
-PACKAGES = {
-    "django.db.backends.postgresql_psycopg2": "psycopg2.extensions",
-    "sentry.db.postgres": "psycopg2.extensions",
-    "sentry.utils.memcached.MemcachedCache": "memcache",
-    "django.core.cache.backends.memcached.PyLibMCCache": "pylibmc",
-}
-
-
-def validate_settings(settings):
-    for key, engine_key, engine_type in [
-        ("DATABASES", "ENGINE", "database engine"),
-        ("CACHES", "BACKEND", "caching backend"),
-    ]:
-
-        value = getattr(settings, key, {})
-        for alias in value:
-            engine = value[alias][engine_key]
-            if engine not in PACKAGES:
-                continue
-            validate_dependency(settings, engine_type, engine, PACKAGES[engine])
-
-
-def validate_dependency(settings, dependency_type, dependency, package):
-    try:
-        import_string(package)
-    except ImportError as e:
-        msg = ConfigurationError.get_error_message(f"{dependency_type} {dependency}", package)
-        raise ConfigurationError(msg).with_traceback(e.__traceback__)
-
-
-class ConfigurationError(ValueError):
-    """
-    This error is thrown whenever a sentry configuration is wrong, or requires a third-party library
-    that's not installed properly or can't be found.
-    """
-
-    @classmethod
-    def get_error_message(cls, dependency, package):
-        return (
-            """Python could not find %(package)s in your current environment (required by %(dependency)s). If you have it installed, maybe you are using the wrong python binary to run sentry?"""
-            % {"dependency": dependency, "package": package}
-        )
-
-
 def is_self_hosted() -> bool:
     # Backcompat for rename to support old consumers, particularly single-tenant.
     from django.conf import settings

+ 0 - 86
tests/sentry/utils/test_settings.py

@@ -1,86 +0,0 @@
-from unittest import mock
-
-import pytest
-from django.conf import settings
-
-from sentry.testutils.cases import TestCase
-from sentry.utils.imports import import_string
-from sentry.utils.settings import ConfigurationError, validate_settings
-
-DEPENDENCY_TEST_DATA = {
-    "postgresql": (
-        "DATABASES",
-        "psycopg2.extensions",
-        "database engine",
-        "django.db.backends.postgresql_psycopg2",
-        {
-            "default": {
-                "ENGINE": "django.db.backends.postgresql_psycopg2",
-                "NAME": "test",
-                "USER": "root",
-                "PASSWORD": "",
-                "HOST": "127.0.0.1",
-                "PORT": "",
-            }
-        },
-    ),
-    "memcache": (
-        "CACHES",
-        "memcache",
-        "caching backend",
-        "django.core.cache.backends.memcached.MemcachedCache",
-        {
-            "default": {
-                "BACKEND": "sentry.utils.memcached.MemcachedCache",
-                "LOCATION": "127.0.0.1:11211",
-            }
-        },
-    ),
-    "pylibmc": (
-        "CACHES",
-        "pylibmc",
-        "caching backend",
-        "django.core.cache.backends.memcached.PyLibMCCache",
-        {
-            "default": {
-                "BACKEND": "django.core.cache.backends.memcached.PyLibMCCache",
-                "LOCATION": "127.0.0.1:11211",
-            }
-        },
-    ),
-}
-
-
-class DependencyTest(TestCase):
-    def raise_import_error(self, package):
-        def callable(package_name):
-            if package_name != package:
-                return import_string(package_name)
-            raise ImportError(f"No module named {package}")
-
-        return callable
-
-    @mock.patch("django.conf.settings", mock.Mock())
-    @mock.patch("sentry.utils.settings.import_string")
-    def validate_dependency(
-        self, key, package, dependency_type, dependency, setting_value, import_string
-    ):
-        import_string.side_effect = self.raise_import_error(package)
-
-        with self.settings(**{key: setting_value}):
-            with pytest.raises(ConfigurationError):
-                validate_settings(settings)
-
-    def test_validate_fails_on_postgres(self):
-        with pytest.warns(UserWarning) as warninfo:
-            self.validate_dependency(*DEPENDENCY_TEST_DATA["postgresql"])
-        (warn,) = warninfo
-        assert isinstance(warn.message, UserWarning)
-        (msg,) = warn.message.args
-        assert msg == "Overriding setting DATABASES can lead to unexpected behavior."
-
-    def test_validate_fails_on_memcache(self):
-        self.validate_dependency(*DEPENDENCY_TEST_DATA["memcache"])
-
-    def test_validate_fails_on_pylibmc(self):
-        self.validate_dependency(*DEPENDENCY_TEST_DATA["pylibmc"])