Remove various bits of compatibility code for Python <3.6 (#9879)

anoadragon453 · web-flow · commit fe604a022a71 · 2021-04-27T13:13:07.000+01:00
I went through and removed a bunch of cruft that was lying around for compatibility with old Python versions. This PR also will now prevent Synapse from starting unless you're running Python 3.6+.
diff --git a/changelog.d/9879.misc b/changelog.d/9879.misc
@@ -0,0 +1 @@
+Remove backwards-compatibility code for Python versions < 3.6.
diff --git a/mypy.ini b/mypy.ini
@@ -41,7 +41,6 @@ files =
   synapse/push,
   synapse/replication,
   synapse/rest,
-  synapse/secrets.py,
   synapse/server.py,
   synapse/server_notices,
   synapse/spam_checker_api,
diff --git a/synapse/__init__.py b/synapse/__init__.py
@@ -21,8 +21,8 @@
 import sys
 
 # Check that we're not running on an unsupported Python version.
-if sys.version_info < (3, 5):
-    print("Synapse requires Python 3.5 or above.")
+if sys.version_info < (3, 6):
+    print("Synapse requires Python 3.6 or above.")
     sys.exit(1)
 
 # Twisted and canonicaljson will fail to import when this file is executed to
diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py
@@ -85,7 +85,7 @@
     "typing-extensions>=3.7.4",
     # We enforce that we have a `cryptography` version that bundles an `openssl`
     # with the latest security patches.
-    "cryptography>=3.4.7;python_version>='3.6'",
+    "cryptography>=3.4.7",
 ]
 
 CONDITIONAL_REQUIREMENTS = {
@@ -100,14 +100,9 @@
     # that use the protocol, such as Let's Encrypt.
     "acme": [
         "txacme>=0.9.2",
-        # txacme depends on eliot. Eliot 1.8.0 is incompatible with
-        # python 3.5.2, as per https://github.com/itamarst/eliot/issues/418
-        "eliot<1.8.0;python_version<'3.5.3'",
     ],
     "saml2": [
-        # pysaml2 6.4.0 is incompatible with Python 3.5 (see https://github.com/IdentityPython/pysaml2/issues/749)
-        "pysaml2>=4.5.0,<6.4.0;python_version<'3.6'",
-        "pysaml2>=4.5.0;python_version>='3.6'",
+        "pysaml2>=4.5.0",
     ],
     "oidc": ["authlib>=0.14.0"],
     # systemd-python is necessary for logging to the systemd journal via
diff --git a/synapse/rest/admin/users.py b/synapse/rest/admin/users.py
@@ -14,6 +14,7 @@
 import hashlib
 import hmac
 import logging
+import secrets
 from http import HTTPStatus
 from typing import TYPE_CHECKING, Dict, List, Optional, Tuple
 
@@ -375,7 +376,7 @@ def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
         """
         self._clear_old_nonces()
 
-        nonce = self.hs.get_secrets().token_hex(64)
+        nonce = secrets.token_hex(64)
         self.nonces[nonce] = int(self.reactor.seconds())
         return 200, {"nonce": nonce}
 
diff --git a/synapse/rest/consent/consent_resource.py b/synapse/rest/consent/consent_resource.py
@@ -32,14 +32,6 @@
 
 logger = logging.getLogger(__name__)
 
-# use hmac.compare_digest if we have it (python 2.7.7), else just use equality
-if hasattr(hmac, "compare_digest"):
-    compare_digest = hmac.compare_digest
-else:
-
-    def compare_digest(a, b):
-        return a == b
-
 
 class ConsentResource(DirectServeHtmlResource):
     """A twisted Resource to display a privacy policy and gather consent to it
@@ -209,5 +201,5 @@ def _check_hash(self, userid, userhmac):
             .encode("ascii")
         )
 
-        if not compare_digest(want_mac, userhmac):
+        if not hmac.compare_digest(want_mac, userhmac):
             raise SynapseError(HTTPStatus.FORBIDDEN, "HMAC incorrect")
diff --git a/synapse/rest/media/v1/filepath.py b/synapse/rest/media/v1/filepath.py
@@ -21,7 +21,7 @@
 NEW_FORMAT_ID_RE = re.compile(r"^\d\d\d\d-\d\d-\d\d")
 
 
-def _wrap_in_base_path(func: "Callable[..., str]") -> "Callable[..., str]":
+def _wrap_in_base_path(func: Callable[..., str]) -> Callable[..., str]:
     """Takes a function that returns a relative path and turns it into an
     absolute path based on the location of the primary media store
     """
diff --git a/synapse/secrets.py b/synapse/secrets.py
diff --git a/synapse/server.py b/synapse/server.py
@@ -126,7 +126,6 @@
     MediaRepository,
     MediaRepositoryResource,
 )
-from synapse.secrets import Secrets
 from synapse.server_notices.server_notices_manager import ServerNoticesManager
 from synapse.server_notices.server_notices_sender import ServerNoticesSender
 from synapse.server_notices.worker_server_notices_sender import (
@@ -641,10 +640,6 @@ def get_groups_attestation_signing(self) -> GroupAttestationSigning:
     def get_groups_attestation_renewer(self) -> GroupAttestionRenewer:
         return GroupAttestionRenewer(self)
 
-    @cache_in_self
-    def get_secrets(self) -> Secrets:
-        return Secrets()
-
     @cache_in_self
     def get_stats_handler(self) -> StatsHandler:
         return StatsHandler(self)
diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py
@@ -114,7 +114,7 @@ def db_to_json(db_content: Union[memoryview, bytes, bytearray, str]) -> Any:
         db_content = db_content.tobytes()
 
     # Decode it to a Unicode string before feeding it to the JSON decoder, since
-    # Python 3.5 does not support deserializing bytes.
+    # it only supports handling strings
     if isinstance(db_content, (bytes, bytearray)):
         db_content = db_content.decode("utf8")
 
diff --git a/synapse/storage/database.py b/synapse/storage/database.py
@@ -171,10 +171,7 @@ def __getattr__(self, name):
 
 
 # The type of entry which goes on our after_callbacks and exception_callbacks lists.
-#
-# Python 3.5.2 doesn't support Callable with an ellipsis, so we wrap it in quotes so
-# that mypy sees the type but the runtime python doesn't.
-_CallbackListEntry = Tuple["Callable[..., None]", Iterable[Any], Dict[str, Any]]
+_CallbackListEntry = Tuple[Callable[..., None], Iterable[Any], Dict[str, Any]]
 
 
 R = TypeVar("R")
@@ -221,7 +218,7 @@ def __init__(
         self.after_callbacks = after_callbacks
         self.exception_callbacks = exception_callbacks
 
-    def call_after(self, callback: "Callable[..., None]", *args: Any, **kwargs: Any):
+    def call_after(self, callback: Callable[..., None], *args: Any, **kwargs: Any):
         """Call the given callback on the main twisted thread after the
         transaction has finished. Used to invalidate the caches on the
         correct thread.
@@ -233,7 +230,7 @@ def call_after(self, callback: "Callable[..., None]", *args: Any, **kwargs: Any)
         self.after_callbacks.append((callback, args, kwargs))
 
     def call_on_exception(
-        self, callback: "Callable[..., None]", *args: Any, **kwargs: Any
+        self, callback: Callable[..., None], *args: Any, **kwargs: Any
     ):
         # if self.exception_callbacks is None, that means that whatever constructed the
         # LoggingTransaction isn't expecting there to be any callbacks; assert that
@@ -485,7 +482,7 @@ def new_transaction(
         desc: str,
         after_callbacks: List[_CallbackListEntry],
         exception_callbacks: List[_CallbackListEntry],
-        func: "Callable[..., R]",
+        func: Callable[..., R],
         *args: Any,
         **kwargs: Any,
     ) -> R:
@@ -618,7 +615,7 @@ def new_transaction(
     async def runInteraction(
         self,
         desc: str,
-        func: "Callable[..., R]",
+        func: Callable[..., R],
         *args: Any,
         db_autocommit: bool = False,
         **kwargs: Any,
@@ -678,7 +675,7 @@ async def runInteraction(
 
     async def runWithConnection(
         self,
-        func: "Callable[..., R]",
+        func: Callable[..., R],
         *args: Any,
         db_autocommit: bool = False,
         **kwargs: Any,
diff --git a/synapse/util/caches/response_cache.py b/synapse/util/caches/response_cache.py
@@ -110,7 +110,7 @@ def remove(r):
         return result.observe()
 
     def wrap(
-        self, key: T, callback: "Callable[..., Any]", *args: Any, **kwargs: Any
+        self, key: T, callback: Callable[..., Any], *args: Any, **kwargs: Any
     ) -> defer.Deferred:
         """Wrap together a *get* and *set* call, taking care of logcontexts
 
diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py
@@ -18,7 +18,7 @@
 import urllib.parse
 from binascii import unhexlify
 from typing import List, Optional
-from unittest.mock import Mock
+from unittest.mock import Mock, patch
 
 import synapse.rest.admin
 from synapse.api.constants import UserTypes
@@ -54,8 +54,6 @@ def make_homeserver(self, reactor, clock):
         self.datastore = Mock(return_value=Mock())
         self.datastore.get_current_state_deltas = Mock(return_value=(0, []))
 
-        self.secrets = Mock()
-
         self.hs = self.setup_test_homeserver()
 
         self.hs.config.registration_shared_secret = "shared"
@@ -84,14 +82,13 @@ def test_get_nonce(self):
         Calling GET on the endpoint will return a randomised nonce, using the
         homeserver's secrets provider.
         """
-        secrets = Mock()
-        secrets.token_hex = Mock(return_value="abcd")
-
-        self.hs.get_secrets = Mock(return_value=secrets)
+        with patch("secrets.token_hex") as token_hex:
+            # Patch secrets.token_hex for the duration of this context
+            token_hex.return_value = "abcd"
 
-        channel = self.make_request("GET", self.url)
+            channel = self.make_request("GET", self.url)
 
-        self.assertEqual(channel.json_body, {"nonce": "abcd"})
+            self.assertEqual(channel.json_body, {"nonce": "abcd"})
 
     def test_expired_nonce(self):
         """
diff --git a/tests/storage/test__base.py b/tests/storage/test__base.py
@@ -13,6 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import secrets
 
 from tests import unittest
 
@@ -21,7 +22,7 @@ class UpsertManyTests(unittest.HomeserverTestCase):
     def prepare(self, reactor, clock, hs):
         self.storage = hs.get_datastore()
 
-        self.table_name = "table_" + hs.get_secrets().token_hex(6)
+        self.table_name = "table_" + secrets.token_hex(6)
         self.get_success(
             self.storage.db_pool.runInteraction(
                 "create",
diff --git a/tests/unittest.py b/tests/unittest.py
@@ -18,6 +18,7 @@
 import hmac
 import inspect
 import logging
+import secrets
 import time
 from typing import Callable, Dict, Iterable, Optional, Tuple, Type, TypeVar, Union
 from unittest.mock import Mock, patch
@@ -626,7 +627,6 @@ def create_and_send_event(
             str: The new event's ID.
         """
         event_creator = self.hs.get_event_creation_handler()
-        secrets = self.hs.get_secrets()
         requester = create_requester(user)
 
         event, context = self.get_success(
diff --git a/tox.ini b/tox.ini
@@ -21,13 +21,11 @@ deps =
     # installed on that).
     #
     # anyway, make sure that we have a recent enough setuptools.
-    setuptools>=18.5 ; python_version >= '3.6'
-    setuptools>=18.5,<51.0.0 ; python_version < '3.6'
+    setuptools>=18.5
 
     # we also need a semi-recent version of pip, because old ones fail to
     # install the "enum34" dependency of cryptography.
-    pip>=10 ; python_version >= '3.6'
-    pip>=10,<21.0 ; python_version < '3.6'
+    pip>=10
 
 # directories/files we run the linters on.
 # if you update this list, make sure to do the same in scripts-dev/lint.sh
@@ -168,8 +166,7 @@ skip_install = true
 usedevelop = false
 deps =
     coverage
-    pip>=10 ; python_version >= '3.6'
-    pip>=10,<21.0 ; python_version < '3.6'
+    pip>=10
 commands=
     coverage combine
     coverage report

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+Remove backwards-compatibility code for Python versions < 3.6.`