From 1f634fad2d903fe8c9ed7f8330b19262b41efe93 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 7 Jun 2022 12:54:52 +0100 Subject: [PATCH 1/5] Use dummy fallback engines if imports fail --- synapse/storage/engines/__init__.py | 38 ++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/synapse/storage/engines/__init__.py b/synapse/storage/engines/__init__.py index f51b3d228ee7..a182e8a098b1 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", +] From 49bec55318575b3915256186841e1963b69a87d4 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 7 Jun 2022 12:55:10 +0100 Subject: [PATCH 2/5] Use this moment to unconditionally import psycopg2 --- synapse/storage/engines/postgres.py | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/synapse/storage/engines/postgres.py b/synapse/storage/engines/postgres.py index 391f8ed24a3d..8e80697ba138 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.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.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,7 @@ def get_db_locale(self, txn: Cursor) -> Tuple[str, str]: return collation, ctype def check_database( - self, db_conn: "psycopg2.connection", allow_outdated_version: bool = False + self, db_conn: psycopg2.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 +174,6 @@ def supports_returning(self) -> bool: 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 +181,7 @@ def is_deadlock(self, error: Exception) -> bool: return error.pgcode in ["40001", "40P01"] return False - def is_connection_closed(self, conn: "psycopg2.connection") -> bool: + def is_connection_closed(self, conn: psycopg2.connection) -> bool: return bool(conn.closed) def lock_table(self, txn: Cursor, table: str) -> None: @@ -205,18 +201,16 @@ def server_version(self) -> str: 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.connection) -> bool: return conn.status != psycopg2.extensions.STATUS_READY def attempt_to_set_autocommit( - self, conn: "psycopg2.connection", autocommit: bool + self, conn: psycopg2.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.connection, isolation_level: Optional[int] ) -> None: if isolation_level is None: isolation_level = self.default_isolation_level From 78d25b4a9854f87b9822c876c141093c44bd391f Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 7 Jun 2022 13:10:57 +0100 Subject: [PATCH 3/5] Changelog --- changelog.d/12979.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12979.bugfix diff --git a/changelog.d/12979.bugfix b/changelog.d/12979.bugfix new file mode 100644 index 000000000000..6b54408025ec --- /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. From 38feea71254d3d228cb5ce7c1008519149a30d36 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 7 Jun 2022 13:17:57 +0100 Subject: [PATCH 4/5] Apparently it's `psycopg2.extensions.connection`? Ooops --- synapse/storage/engines/postgres.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/synapse/storage/engines/postgres.py b/synapse/storage/engines/postgres.py index 8e80697ba138..517f9d5f98d7 100644 --- a/synapse/storage/engines/postgres.py +++ b/synapse/storage/engines/postgres.py @@ -31,7 +31,7 @@ logger = logging.getLogger(__name__) -class PostgresEngine(BaseDatabaseEngine[psycopg2.connection]): +class PostgresEngine(BaseDatabaseEngine[psycopg2.extensions.connection]): def __init__(self, database_config: Mapping[str, Any]): super().__init__(psycopg2, database_config) psycopg2.extensions.register_type(psycopg2.extensions.UNICODE) @@ -67,7 +67,9 @@ def get_db_locale(self, txn: Cursor) -> Tuple[str, str]: 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 @@ -181,7 +183,7 @@ def is_deadlock(self, error: Exception) -> bool: 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: @@ -201,16 +203,16 @@ def server_version(self) -> str: else: return "%i.%i.%i" % (numver / 10000, (numver % 10000) / 100, numver % 100) - def in_transaction(self, conn: psycopg2.connection) -> bool: + 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 From 470ec11be6d0ae896bdd89a88e4e6647d2945e14 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 7 Jun 2022 13:30:13 +0100 Subject: [PATCH 5/5] Always import from engines.__init__ --- synapse/storage/databases/main/events.py | 2 +- synapse/storage/prepare_database.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index 17e35cf63e68..a8773374be9c 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -46,7 +46,7 @@ ) 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/prepare_database.py b/synapse/storage/prepare_database.py index c33df420841d..09a2b58f4c72 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -23,8 +23,7 @@ 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