summary refs log tree commit diff
diff options
context:
space:
mode:
authorDavid Robertson <davidr@element.io>2022-06-07 17:33:55 +0100
committerGitHub <noreply@github.com>2022-06-07 17:33:55 +0100
commit586bfc6dc0241bdd40376c3314f13b82a5593538 (patch)
tree2ab1a8b5cfaa5745e7b9db1f09009051e0d432c7
parentTest cancellation at every `await` during request handling (#12674) (diff)
downloadsynapse-586bfc6dc0241bdd40376c3314f13b82a5593538.tar.xz
Use dummy fallback engines if imports fail (#12979)
-rw-r--r--changelog.d/12979.bugfix1
-rw-r--r--synapse/storage/databases/main/events.py2
-rw-r--r--synapse/storage/engines/__init__.py38
-rw-r--r--synapse/storage/engines/postgres.py24
-rw-r--r--synapse/storage/prepare_database.py3
5 files changed, 47 insertions, 21 deletions
diff --git a/changelog.d/12979.bugfix b/changelog.d/12979.bugfix
new file mode 100644
index 0000000000..6b54408025
--- /dev/null
+++ b/changelog.d/12979.bugfix
@@ -0,0 +1 @@
+Fix a bug introduced in Synapse 1.60 where Synapse would fail to start if the `sqlite3` module was not available.
diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py
index 17e35cf63e..a8773374be 100644
--- a/synapse/storage/databases/main/events.py
+++ b/synapse/storage/databases/main/events.py
@@ -46,7 +46,7 @@ from synapse.storage.database import (
 )
 from synapse.storage.databases.main.events_worker import EventCacheEntry
 from synapse.storage.databases.main.search import SearchEntry
-from synapse.storage.engines.postgres import PostgresEngine
+from synapse.storage.engines import PostgresEngine
 from synapse.storage.util.id_generators import AbstractStreamIdGenerator
 from synapse.storage.util.sequence import SequenceGenerator
 from synapse.types import JsonDict, StateMap, get_domain_from_id
diff --git a/synapse/storage/engines/__init__.py b/synapse/storage/engines/__init__.py
index f51b3d228e..a182e8a098 100644
--- a/synapse/storage/engines/__init__.py
+++ b/synapse/storage/engines/__init__.py
@@ -11,11 +11,35 @@
 # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 # See the License for the specific language governing permissions and
 # limitations under the License.
-from typing import Any, Mapping
+from typing import Any, Mapping, NoReturn
 
 from ._base import BaseDatabaseEngine, IncorrectDatabaseSetup
-from .postgres import PostgresEngine
-from .sqlite import Sqlite3Engine
+
+# The classes `PostgresEngine` and `Sqlite3Engine` must always be importable, because
+# we use `isinstance(engine, PostgresEngine)` to write different queries for postgres
+# and sqlite. But the database driver modules are both optional: they may not be
+# installed. To account for this, create dummy classes on import failure so we can
+# still run `isinstance()` checks.
+try:
+    from .postgres import PostgresEngine
+except ImportError:
+
+    class PostgresEngine(BaseDatabaseEngine):  # type: ignore[no-redef]
+        def __new__(cls, *args: object, **kwargs: object) -> NoReturn:  # type: ignore[misc]
+            raise RuntimeError(
+                f"Cannot create {cls.__name__} -- psycopg2 module is not installed"
+            )
+
+
+try:
+    from .sqlite import Sqlite3Engine
+except ImportError:
+
+    class Sqlite3Engine(BaseDatabaseEngine):  # type: ignore[no-redef]
+        def __new__(cls, *args: object, **kwargs: object) -> NoReturn:  # type: ignore[misc]
+            raise RuntimeError(
+                f"Cannot create {cls.__name__} -- sqlite3 module is not installed"
+            )
 
 
 def create_engine(database_config: Mapping[str, Any]) -> BaseDatabaseEngine:
@@ -30,4 +54,10 @@ def create_engine(database_config: Mapping[str, Any]) -> BaseDatabaseEngine:
     raise RuntimeError("Unsupported database engine '%s'" % (name,))
 
 
-__all__ = ["create_engine", "BaseDatabaseEngine", "IncorrectDatabaseSetup"]
+__all__ = [
+    "create_engine",
+    "BaseDatabaseEngine",
+    "PostgresEngine",
+    "Sqlite3Engine",
+    "IncorrectDatabaseSetup",
+]
diff --git a/synapse/storage/engines/postgres.py b/synapse/storage/engines/postgres.py
index 391f8ed24a..517f9d5f98 100644
--- a/synapse/storage/engines/postgres.py
+++ b/synapse/storage/engines/postgres.py
@@ -15,6 +15,8 @@
 import logging
 from typing import TYPE_CHECKING, Any, Mapping, NoReturn, Optional, Tuple, cast
 
+import psycopg2.extensions
+
 from synapse.storage.engines._base import (
     BaseDatabaseEngine,
     IncorrectDatabaseSetup,
@@ -23,18 +25,14 @@ from synapse.storage.engines._base import (
 from synapse.storage.types import Cursor
 
 if TYPE_CHECKING:
-    import psycopg2  # noqa: F401
-
     from synapse.storage.database import LoggingDatabaseConnection
 
 
 logger = logging.getLogger(__name__)
 
 
-class PostgresEngine(BaseDatabaseEngine["psycopg2.connection"]):
+class PostgresEngine(BaseDatabaseEngine[psycopg2.extensions.connection]):
     def __init__(self, database_config: Mapping[str, Any]):
-        import psycopg2.extensions
-
         super().__init__(psycopg2, database_config)
         psycopg2.extensions.register_type(psycopg2.extensions.UNICODE)
 
@@ -69,7 +67,9 @@ class PostgresEngine(BaseDatabaseEngine["psycopg2.connection"]):
         return collation, ctype
 
     def check_database(
-        self, db_conn: "psycopg2.connection", allow_outdated_version: bool = False
+        self,
+        db_conn: psycopg2.extensions.connection,
+        allow_outdated_version: bool = False,
     ) -> None:
         # Get the version of PostgreSQL that we're using. As per the psycopg2
         # docs: The number is formed by converting the major, minor, and
@@ -176,8 +176,6 @@ class PostgresEngine(BaseDatabaseEngine["psycopg2.connection"]):
         return True
 
     def is_deadlock(self, error: Exception) -> bool:
-        import psycopg2.extensions
-
         if isinstance(error, psycopg2.DatabaseError):
             # https://www.postgresql.org/docs/current/static/errcodes-appendix.html
             # "40001" serialization_failure
@@ -185,7 +183,7 @@ class PostgresEngine(BaseDatabaseEngine["psycopg2.connection"]):
             return error.pgcode in ["40001", "40P01"]
         return False
 
-    def is_connection_closed(self, conn: "psycopg2.connection") -> bool:
+    def is_connection_closed(self, conn: psycopg2.extensions.connection) -> bool:
         return bool(conn.closed)
 
     def lock_table(self, txn: Cursor, table: str) -> None:
@@ -205,18 +203,16 @@ class PostgresEngine(BaseDatabaseEngine["psycopg2.connection"]):
         else:
             return "%i.%i.%i" % (numver / 10000, (numver % 10000) / 100, numver % 100)
 
-    def in_transaction(self, conn: "psycopg2.connection") -> bool:
-        import psycopg2.extensions
-
+    def in_transaction(self, conn: psycopg2.extensions.connection) -> bool:
         return conn.status != psycopg2.extensions.STATUS_READY
 
     def attempt_to_set_autocommit(
-        self, conn: "psycopg2.connection", autocommit: bool
+        self, conn: psycopg2.extensions.connection, autocommit: bool
     ) -> None:
         return conn.set_session(autocommit=autocommit)
 
     def attempt_to_set_isolation_level(
-        self, conn: "psycopg2.connection", isolation_level: Optional[int]
+        self, conn: psycopg2.extensions.connection, isolation_level: Optional[int]
     ) -> None:
         if isolation_level is None:
             isolation_level = self.default_isolation_level
diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py
index c33df42084..09a2b58f4c 100644
--- a/synapse/storage/prepare_database.py
+++ b/synapse/storage/prepare_database.py
@@ -23,8 +23,7 @@ from typing_extensions import Counter as CounterType
 
 from synapse.config.homeserver import HomeServerConfig
 from synapse.storage.database import LoggingDatabaseConnection
-from synapse.storage.engines import BaseDatabaseEngine
-from synapse.storage.engines.postgres import PostgresEngine
+from synapse.storage.engines import BaseDatabaseEngine, PostgresEngine
 from synapse.storage.schema import SCHEMA_COMPAT_VERSION, SCHEMA_VERSION
 from synapse.storage.types import Cursor