From 8cd73b2c16d76b3a5322342fc6155d1567c242cd Mon Sep 17 00:00:00 2001 From: Vitaly Samigullin Date: Tue, 21 Nov 2023 18:53:53 +0100 Subject: [PATCH 01/10] Make testing infra based on testcontainers framework-agnostic --- .../testing/testcontainers/cratedb.py | 99 +++++++++++++++++-- pyproject.toml | 2 + tests/conftest.py | 55 +---------- 3 files changed, 98 insertions(+), 58 deletions(-) diff --git a/cratedb_toolkit/testing/testcontainers/cratedb.py b/cratedb_toolkit/testing/testcontainers/cratedb.py index 923c197..5ec4d0a 100644 --- a/cratedb_toolkit/testing/testcontainers/cratedb.py +++ b/cratedb_toolkit/testing/testcontainers/cratedb.py @@ -19,6 +19,7 @@ from testcontainers.core.waiting_utils import wait_container_is_ready, wait_for_logs from cratedb_toolkit.testing.testcontainers.util import KeepaliveContainer, asbool +from cratedb_toolkit.util import DatabaseAdapter logger = logging.getLogger(__name__) @@ -51,8 +52,13 @@ class CrateDBContainer(KeepaliveContainer, DbContainer): CRATEDB_PASSWORD = os.environ.get("CRATEDB_PASSWORD", "") CRATEDB_DB = os.environ.get("CRATEDB_DB", "doc") KEEPALIVE = asbool(os.environ.get("CRATEDB_KEEPALIVE", os.environ.get("TC_KEEPALIVE", False))) + CMD_OPTS = { + "discovery.type": "single-node", + "cluster.routing.allocation.disk.threshold_enabled": False, + "node.attr.storage": "hot", + "path.repo": "/tmp/snapshots", + } - # TODO: Dual-port use with 4200+5432. def __init__( self, image: str = "crate/crate:nightly", @@ -61,34 +67,49 @@ def __init__( password: Optional[str] = None, dbname: Optional[str] = None, dialect: str = "crate", + cmd_opts: Optional[dict] = None, + extra_ports: Optional[list] = None, **kwargs, ) -> None: super().__init__(image=image, **kwargs) self._name = "testcontainers-cratedb" # -{os.getpid()} - self._command = "-Cdiscovery.type=single-node -Ccluster.routing.allocation.disk.threshold_enabled=false" - # TODO: Generalize by obtaining more_opts from caller. - self._command += " -Cnode.attr.storage=hot" - self._command += " -Cpath.repo=/tmp/snapshots" + + cmd_opts = cmd_opts if cmd_opts else {} + self._command = self._build_cmd({**self.CMD_OPTS, **cmd_opts}) self.CRATEDB_USER = user or self.CRATEDB_USER self.CRATEDB_PASSWORD = password or self.CRATEDB_PASSWORD self.CRATEDB_DB = dbname or self.CRATEDB_DB self.port_to_expose = port + self.extra_ports = extra_ports or [] self.dialect = dialect + @staticmethod + def _build_cmd(opts: dict) -> str: + """ + Return a string with command options concatenated and optimised for ES5 use + """ + cmd = [] + for key, val in opts.items(): + if isinstance(val, bool): + val = str(val).lower() + cmd.append("-C{}={}".format(key, val)) + return " ".join(cmd) + def _configure(self) -> None: - self.with_exposed_ports(self.port_to_expose) + ports = [*[self.port_to_expose], *self.extra_ports] + self.with_exposed_ports(*ports) self.with_env("CRATEDB_USER", self.CRATEDB_USER) self.with_env("CRATEDB_PASSWORD", self.CRATEDB_PASSWORD) self.with_env("CRATEDB_DB", self.CRATEDB_DB) - def get_connection_url(self, host=None) -> str: + def get_connection_url(self, host=None, dialect=None) -> str: # TODO: When using `db_name=self.CRATEDB_DB`: # Connection.__init__() got an unexpected keyword argument 'database' return super()._create_connection_url( - dialect=self.dialect, + dialect=dialect or self.dialect, username=self.CRATEDB_USER, password=self.CRATEDB_PASSWORD, host=host, @@ -101,3 +122,65 @@ def _connect(self): # In `testcontainers-java`, there is the `HttpWaitStrategy`. # TODO: Provide a client instance. wait_for_logs(self, predicate="o.e.n.Node.*started", timeout=MAX_TRIES) + + +class TestDrive: + """ + Use different schemas for storing the subsystem database tables, and the + test/example data, so that they do not accidentally touch the default `doc` + schema. + """ + + EXT_SCHEMA = "testdrive-ext" + DATA_SCHEMA = "testdrive-data" + + RESET_TABLES = [ + f'"{EXT_SCHEMA}"."retention_policy"', + f'"{DATA_SCHEMA}"."raw_metrics"', + f'"{DATA_SCHEMA}"."sensor_readings"', + f'"{DATA_SCHEMA}"."testdrive"', + f'"{DATA_SCHEMA}"."foobar"', + f'"{DATA_SCHEMA}"."foobar_unique_single"', + f'"{DATA_SCHEMA}"."foobar_unique_composite"', + # cratedb_toolkit.io.{influxdb,mongodb} + '"testdrive"."demo"', + ] + + +class CrateDBFixture: + """ + A little helper wrapping Testcontainer's `CrateDBContainer` and + CrateDB Toolkit's `DatabaseAdapter`, agnostic of the test framework. + """ + + def __init__(self, crate_version: str = "nightly"): + self.cratedb: Optional[CrateDBContainer] = None + self.image: str = "crate/crate:{}".format(crate_version) + self.database: Optional[DatabaseAdapter] = None + self.setup() + + def setup(self): + self.cratedb = CrateDBContainer(image=self.image) + self.cratedb.start() + self.database = DatabaseAdapter(dburi=self.get_connection_url()) + + def finalize(self): + if self.cratedb: + self.cratedb.stop() + + def reset(self, tables: Optional[str] = None): + if tables and self.database: + for reset_table in tables: + self.database.connection.exec_driver_sql(f"DROP TABLE IF EXISTS {reset_table};") + + def get_connection_url(self, *args, **kwargs): + if self.cratedb: + return self.cratedb.get_connection_url(*args, **kwargs) + return None + + @property + def http_url(self): + """ + Return a URL for HTTP interface + """ + return self.get_connection_url(dialect="http") diff --git a/pyproject.toml b/pyproject.toml index 2d5d4be..02d4f5a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -235,6 +235,8 @@ extend-ignore = [ "RET504", # Unnecessary `elif` after `return` statement "RET505", + # Probable insecure usage of temporary file or directory + "S108", ] extend-exclude = [ diff --git a/tests/conftest.py b/tests/conftest.py index c2f4512..1622884 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,56 +3,11 @@ import pytest import responses -from cratedb_toolkit.testing.testcontainers.cratedb import CrateDBContainer -from cratedb_toolkit.util import DatabaseAdapter +from cratedb_toolkit.testing.testcontainers.cratedb import CrateDBFixture, TestDrive from cratedb_toolkit.util.common import setup_logging -# Use different schemas for storing the subsystem database tables, and the -# test/example data, so that they do not accidentally touch the default `doc` -# schema. -TESTDRIVE_EXT_SCHEMA = "testdrive-ext" -TESTDRIVE_DATA_SCHEMA = "testdrive-data" - -RESET_TABLES = [ - f'"{TESTDRIVE_EXT_SCHEMA}"."retention_policy"', - f'"{TESTDRIVE_DATA_SCHEMA}"."raw_metrics"', - f'"{TESTDRIVE_DATA_SCHEMA}"."sensor_readings"', - f'"{TESTDRIVE_DATA_SCHEMA}"."testdrive"', - f'"{TESTDRIVE_DATA_SCHEMA}"."foobar"', - f'"{TESTDRIVE_DATA_SCHEMA}"."foobar_unique_single"', - f'"{TESTDRIVE_DATA_SCHEMA}"."foobar_unique_composite"', - # cratedb_toolkit.io.{influxdb,mongodb} - '"testdrive"."demo"', -] - - -class CrateDBFixture: - """ - A little helper wrapping Testcontainer's `CrateDBContainer` and - CrateDB Toolkit's `DatabaseAdapter`, agnostic of the test framework. - """ - - def __init__(self): - self.cratedb = None - self.database: DatabaseAdapter = None - self.setup() - - def setup(self): - # TODO: Make image name configurable. - self.cratedb = CrateDBContainer("crate/crate:nightly") - self.cratedb.start() - self.database = DatabaseAdapter(dburi=self.get_connection_url()) - - def finalize(self): - self.cratedb.stop() - - def reset(self): - # TODO: Make list of tables configurable. - for reset_table in RESET_TABLES: - self.database.connection.exec_driver_sql(f"DROP TABLE IF EXISTS {reset_table};") - - def get_connection_url(self, *args, **kwargs): - return self.cratedb.get_connection_url(*args, **kwargs) +TESTDRIVE_DATA_SCHEMA = TestDrive.DATA_SCHEMA +TESTDRIVE_EXT_SCHEMA = TestDrive.EXT_SCHEMA @pytest.fixture(scope="session", autouse=True) @@ -63,7 +18,7 @@ def configure_database_schema(session_mocker): If not configured otherwise, the test suite currently uses `testdrive-ext`. """ - session_mocker.patch("os.environ", {"CRATEDB_EXT_SCHEMA": TESTDRIVE_EXT_SCHEMA}) + session_mocker.patch("os.environ", {"CRATEDB_EXT_SCHEMA": TestDrive.EXT_SCHEMA}) @pytest.fixture(scope="session") @@ -82,7 +37,7 @@ def cratedb(cratedb_service): """ Provide a fresh canvas to each test case invocation, by resetting database content. """ - cratedb_service.reset() + cratedb_service.reset(tables=TestDrive.RESET_TABLES) yield cratedb_service From 40bdc7e7828075cd3145f2e745cb5ea702a48f64 Mon Sep 17 00:00:00 2001 From: Vitaly Samigullin Date: Tue, 21 Nov 2023 19:15:29 +0100 Subject: [PATCH 02/10] Allow to pass extra kwargs to CrateDBFixture and CrateDBContainer --- cratedb_toolkit/testing/testcontainers/cratedb.py | 10 +++++----- tests/conftest.py | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/cratedb_toolkit/testing/testcontainers/cratedb.py b/cratedb_toolkit/testing/testcontainers/cratedb.py index 5ec4d0a..c3e3e08 100644 --- a/cratedb_toolkit/testing/testcontainers/cratedb.py +++ b/cratedb_toolkit/testing/testcontainers/cratedb.py @@ -153,14 +153,14 @@ class CrateDBFixture: CrateDB Toolkit's `DatabaseAdapter`, agnostic of the test framework. """ - def __init__(self, crate_version: str = "nightly"): + def __init__(self, crate_version: str = "nightly", **kwargs): self.cratedb: Optional[CrateDBContainer] = None self.image: str = "crate/crate:{}".format(crate_version) self.database: Optional[DatabaseAdapter] = None - self.setup() + self.setup(**kwargs) - def setup(self): - self.cratedb = CrateDBContainer(image=self.image) + def setup(self, **kwargs): + self.cratedb = CrateDBContainer(image=self.image, **kwargs) self.cratedb.start() self.database = DatabaseAdapter(dburi=self.get_connection_url()) @@ -168,7 +168,7 @@ def finalize(self): if self.cratedb: self.cratedb.stop() - def reset(self, tables: Optional[str] = None): + def reset(self, tables: Optional[list] = TestDrive.RESET_TABLES): if tables and self.database: for reset_table in tables: self.database.connection.exec_driver_sql(f"DROP TABLE IF EXISTS {reset_table};") diff --git a/tests/conftest.py b/tests/conftest.py index 1622884..e4784c2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -37,7 +37,7 @@ def cratedb(cratedb_service): """ Provide a fresh canvas to each test case invocation, by resetting database content. """ - cratedb_service.reset(tables=TestDrive.RESET_TABLES) + cratedb_service.reset() yield cratedb_service From 4a1c910e92c9ddce7ca33e09b3c2daacf21ad159 Mon Sep 17 00:00:00 2001 From: Vitaly Samigullin Date: Tue, 21 Nov 2023 22:33:33 +0100 Subject: [PATCH 03/10] Bind port inside container to the same port on the host with testcontainers --- cratedb_toolkit/testing/testcontainers/cratedb.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/cratedb_toolkit/testing/testcontainers/cratedb.py b/cratedb_toolkit/testing/testcontainers/cratedb.py index c3e3e08..f50d224 100644 --- a/cratedb_toolkit/testing/testcontainers/cratedb.py +++ b/cratedb_toolkit/testing/testcontainers/cratedb.py @@ -98,9 +98,18 @@ def _build_cmd(opts: dict) -> str: cmd.append("-C{}={}".format(key, val)) return " ".join(cmd) - def _configure(self) -> None: + def _configure_ports(self) -> None: + """ + Bind all the ports exposed inside the container to the same port on the host + """ ports = [*[self.port_to_expose], *self.extra_ports] - self.with_exposed_ports(*ports) + for port in ports: + # If random port is needed on the host, use host=None + # or invoke self.with_exposed_ports + self.with_bind_ports(container=port, host=port) + + def _configure(self) -> None: + self._configure_ports() self.with_env("CRATEDB_USER", self.CRATEDB_USER) self.with_env("CRATEDB_PASSWORD", self.CRATEDB_PASSWORD) self.with_env("CRATEDB_DB", self.CRATEDB_DB) From 37708497e396829d21fe84ccfc91a66921e4c642 Mon Sep 17 00:00:00 2001 From: Vitaly Samigullin Date: Tue, 21 Nov 2023 23:06:05 +0100 Subject: [PATCH 04/10] Fix generic types for Python 3.7 and 3.8 --- cratedb_toolkit/testing/testcontainers/cratedb.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cratedb_toolkit/testing/testcontainers/cratedb.py b/cratedb_toolkit/testing/testcontainers/cratedb.py index f50d224..8b0d727 100644 --- a/cratedb_toolkit/testing/testcontainers/cratedb.py +++ b/cratedb_toolkit/testing/testcontainers/cratedb.py @@ -10,6 +10,10 @@ # WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the # License for the specific language governing permissions and limitations # under the License. +# This is for Python 3.7 and 3.8 to support generic types +# like `dict` instead of `typing.Dict +from __future__ import annotations + import logging import os from typing import Optional From 8bad2751069da877a79251d0c1d8c1f76b58a02f Mon Sep 17 00:00:00 2001 From: Vitaly Samigullin Date: Wed, 22 Nov 2023 15:23:15 +0100 Subject: [PATCH 05/10] Code review fixes --- .../testing/testcontainers/cratedb.py | 48 +++++++------------ doc/sandbox.md | 2 +- tests/conftest.py | 33 ++++++++++--- 3 files changed, 45 insertions(+), 38 deletions(-) diff --git a/cratedb_toolkit/testing/testcontainers/cratedb.py b/cratedb_toolkit/testing/testcontainers/cratedb.py index 8b0d727..6215b60 100644 --- a/cratedb_toolkit/testing/testcontainers/cratedb.py +++ b/cratedb_toolkit/testing/testcontainers/cratedb.py @@ -58,7 +58,6 @@ class CrateDBContainer(KeepaliveContainer, DbContainer): KEEPALIVE = asbool(os.environ.get("CRATEDB_KEEPALIVE", os.environ.get("TC_KEEPALIVE", False))) CMD_OPTS = { "discovery.type": "single-node", - "cluster.routing.allocation.disk.threshold_enabled": False, "node.attr.storage": "hot", "path.repo": "/tmp/snapshots", } @@ -77,7 +76,7 @@ def __init__( ) -> None: super().__init__(image=image, **kwargs) - self._name = "testcontainers-cratedb" # -{os.getpid()} + self._name = "testcontainers-cratedb" cmd_opts = cmd_opts if cmd_opts else {} self._command = self._build_cmd({**self.CMD_OPTS, **cmd_opts}) @@ -137,29 +136,6 @@ def _connect(self): wait_for_logs(self, predicate="o.e.n.Node.*started", timeout=MAX_TRIES) -class TestDrive: - """ - Use different schemas for storing the subsystem database tables, and the - test/example data, so that they do not accidentally touch the default `doc` - schema. - """ - - EXT_SCHEMA = "testdrive-ext" - DATA_SCHEMA = "testdrive-data" - - RESET_TABLES = [ - f'"{EXT_SCHEMA}"."retention_policy"', - f'"{DATA_SCHEMA}"."raw_metrics"', - f'"{DATA_SCHEMA}"."sensor_readings"', - f'"{DATA_SCHEMA}"."testdrive"', - f'"{DATA_SCHEMA}"."foobar"', - f'"{DATA_SCHEMA}"."foobar_unique_single"', - f'"{DATA_SCHEMA}"."foobar_unique_composite"', - # cratedb_toolkit.io.{influxdb,mongodb} - '"testdrive"."demo"', - ] - - class CrateDBFixture: """ A little helper wrapping Testcontainer's `CrateDBContainer` and @@ -168,25 +144,37 @@ class CrateDBFixture: def __init__(self, crate_version: str = "nightly", **kwargs): self.cratedb: Optional[CrateDBContainer] = None - self.image: str = "crate/crate:{}".format(crate_version) self.database: Optional[DatabaseAdapter] = None - self.setup(**kwargs) + self.image: str = "crate/crate:{}".format(crate_version) - def setup(self, **kwargs): + def start(self, **kwargs): + """ + Start testcontainer, used for tests set up + """ + logger.debug("Starting container % with args %", self.image, **kwargs) self.cratedb = CrateDBContainer(image=self.image, **kwargs) self.cratedb.start() self.database = DatabaseAdapter(dburi=self.get_connection_url()) - def finalize(self): + def stop(self): + """ + Stop testcontainer, used for tests tear down + """ if self.cratedb: self.cratedb.stop() - def reset(self, tables: Optional[list] = TestDrive.RESET_TABLES): + def reset(self, tables: Optional[list] = None): + """ + Drop tables from the given list, used for tests set up or tear down + """ if tables and self.database: for reset_table in tables: self.database.connection.exec_driver_sql(f"DROP TABLE IF EXISTS {reset_table};") def get_connection_url(self, *args, **kwargs): + """ + Return a URL for SQLAlchemy DB engine + """ if self.cratedb: return self.cratedb.get_connection_url(*args, **kwargs) return None diff --git a/doc/sandbox.md b/doc/sandbox.md index 03ebec2..1178d60 100644 --- a/doc/sandbox.md +++ b/doc/sandbox.md @@ -18,7 +18,7 @@ source .venv/bin/activate Install project in sandbox mode. ```shell -pip install --editable='.[develop,test]' +pip install --editable='.[io,test,develop]' ``` Run tests. `TC_KEEPALIVE` keeps the auxiliary service containers running, which diff --git a/tests/conftest.py b/tests/conftest.py index e4784c2..d1afd3a 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,11 +3,29 @@ import pytest import responses -from cratedb_toolkit.testing.testcontainers.cratedb import CrateDBFixture, TestDrive +from cratedb_toolkit.testing.testcontainers.cratedb import CrateDBFixture from cratedb_toolkit.util.common import setup_logging -TESTDRIVE_DATA_SCHEMA = TestDrive.DATA_SCHEMA -TESTDRIVE_EXT_SCHEMA = TestDrive.EXT_SCHEMA +# Use different schemas for storing the subsystem database tables, and the +# test/example data, so that they do not accidentally touch the default `doc` +# schema. + +TESTDRIVE_DATA_SCHEMA = "testdrive-data" +TESTDRIVE_EXT_SCHEMA = "testdrive-ext" +RESET_TABLES = [ + f'"{TESTDRIVE_EXT_SCHEMA}"."retention_policy"', + f'"{TESTDRIVE_DATA_SCHEMA}"."raw_metrics"', + f'"{TESTDRIVE_DATA_SCHEMA}"."sensor_readings"', + f'"{TESTDRIVE_DATA_SCHEMA}"."testdrive"', + f'"{TESTDRIVE_DATA_SCHEMA}"."foobar"', + f'"{TESTDRIVE_DATA_SCHEMA}"."foobar_unique_single"', + f'"{TESTDRIVE_DATA_SCHEMA}"."foobar_unique_composite"', + # cratedb_toolkit.io.{influxdb,mongodb} + '"testdrive"."demo"', +] + +CRATEDB_HTTP_PORT = 44209 +CRATEDB_SETTINGS = {"http.port": CRATEDB_HTTP_PORT} @pytest.fixture(scope="session", autouse=True) @@ -18,7 +36,7 @@ def configure_database_schema(session_mocker): If not configured otherwise, the test suite currently uses `testdrive-ext`. """ - session_mocker.patch("os.environ", {"CRATEDB_EXT_SCHEMA": TestDrive.EXT_SCHEMA}) + session_mocker.patch("os.environ", {"CRATEDB_EXT_SCHEMA": TESTDRIVE_EXT_SCHEMA}) @pytest.fixture(scope="session") @@ -27,9 +45,10 @@ def cratedb_service(): Provide a CrateDB service instance to the test suite. """ db = CrateDBFixture() - db.reset() + db.start(port=CRATEDB_HTTP_PORT, cmd_opts=CRATEDB_SETTINGS) + db.reset(tables=RESET_TABLES) yield db - db.finalize() + db.stop() @pytest.fixture(scope="function") @@ -37,7 +56,7 @@ def cratedb(cratedb_service): """ Provide a fresh canvas to each test case invocation, by resetting database content. """ - cratedb_service.reset() + cratedb_service.reset(tables=RESET_TABLES) yield cratedb_service From 59a548480010c9a3703745f77ad2d3ebedaf663d Mon Sep 17 00:00:00 2001 From: Vitaly Samigullin Date: Wed, 22 Nov 2023 15:25:03 +0100 Subject: [PATCH 06/10] Remove logger --- cratedb_toolkit/testing/testcontainers/cratedb.py | 1 - 1 file changed, 1 deletion(-) diff --git a/cratedb_toolkit/testing/testcontainers/cratedb.py b/cratedb_toolkit/testing/testcontainers/cratedb.py index 6215b60..b5c80f6 100644 --- a/cratedb_toolkit/testing/testcontainers/cratedb.py +++ b/cratedb_toolkit/testing/testcontainers/cratedb.py @@ -151,7 +151,6 @@ def start(self, **kwargs): """ Start testcontainer, used for tests set up """ - logger.debug("Starting container % with args %", self.image, **kwargs) self.cratedb = CrateDBContainer(image=self.image, **kwargs) self.cratedb.start() self.database = DatabaseAdapter(dburi=self.get_connection_url()) From 7924b0d05312cf9592770928d4b82acb293cf9e2 Mon Sep 17 00:00:00 2001 From: Vitaly Samigullin Date: Thu, 23 Nov 2023 00:16:48 +0100 Subject: [PATCH 07/10] Allow binding random ports on host for testcontainers --- .../testing/testcontainers/cratedb.py | 38 +++++++++++++------ tests/conftest.py | 3 +- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/cratedb_toolkit/testing/testcontainers/cratedb.py b/cratedb_toolkit/testing/testcontainers/cratedb.py index b5c80f6..0d5298b 100644 --- a/cratedb_toolkit/testing/testcontainers/cratedb.py +++ b/cratedb_toolkit/testing/testcontainers/cratedb.py @@ -43,6 +43,7 @@ class CrateDBContainer(KeepaliveContainer, DbContainer): >>> import sqlalchemy >>> cratedb_container = CrateDBContainer("crate:5.2.3") + >>> cratedb_container.start() >>> with cratedb_container as cratedb: ... engine = sqlalchemy.create_engine(cratedb.get_connection_url()) ... with engine.begin() as connection: @@ -65,15 +66,29 @@ class CrateDBContainer(KeepaliveContainer, DbContainer): def __init__( self, image: str = "crate/crate:nightly", - port: int = 4200, + ports: Optional[dict] = None, user: Optional[str] = None, password: Optional[str] = None, dbname: Optional[str] = None, dialect: str = "crate", cmd_opts: Optional[dict] = None, - extra_ports: Optional[list] = None, **kwargs, ) -> None: + """ + :param image: docker hub image path with optional tag + :param ports: optional dict that maps a port inside the container to a port on the host machine; + `None` as a map value generates a random port; + Dicts are ordered. By convention the first key-val pair is designated to the HTTP interface. + Example: {4200: None, 5432: 15431} - port 4200 inside the container will be mapped + to a random port on the host, internal port 5432 for PSQL interface will be mapped + to the 15432 port on the host. + :param user: optional username to access the DB; if None, try respective environment variable + :param password: optional password to access the DB; if None, try respective environment variable + :param dbname: optional database name to access the DB; if None, try respective environment variable + :param dialect: a string with the dialect to generate a DB URI + :param cmd_opts: an optional dict with CLI arguments to be passed to the DB entrypoint inside the container + :param kwargs: misc keyword arguments + """ super().__init__(image=image, **kwargs) self._name = "testcontainers-cratedb" @@ -85,8 +100,8 @@ def __init__( self.CRATEDB_PASSWORD = password or self.CRATEDB_PASSWORD self.CRATEDB_DB = dbname or self.CRATEDB_DB - self.port_to_expose = port - self.extra_ports = extra_ports or [] + self.port_mapping = ports if ports else {4200: None} + self.port_to_expose, _ = list(self.port_mapping.items())[0] self.dialect = dialect @staticmethod @@ -105,18 +120,19 @@ def _configure_ports(self) -> None: """ Bind all the ports exposed inside the container to the same port on the host """ - ports = [*[self.port_to_expose], *self.extra_ports] - for port in ports: - # If random port is needed on the host, use host=None - # or invoke self.with_exposed_ports - self.with_bind_ports(container=port, host=port) + # If host_port is `None`, a random port to be generated + for container_port, host_port in self.port_mapping.items(): + self.with_bind_ports(container=container_port, host=host_port) - def _configure(self) -> None: - self._configure_ports() + def _configure_credentials(self) -> None: self.with_env("CRATEDB_USER", self.CRATEDB_USER) self.with_env("CRATEDB_PASSWORD", self.CRATEDB_PASSWORD) self.with_env("CRATEDB_DB", self.CRATEDB_DB) + def _configure(self) -> None: + self._configure_ports() + self._configure_credentials() + def get_connection_url(self, host=None, dialect=None) -> str: # TODO: When using `db_name=self.CRATEDB_DB`: # Connection.__init__() got an unexpected keyword argument 'database' diff --git a/tests/conftest.py b/tests/conftest.py index d1afd3a..181d1a8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -9,7 +9,6 @@ # Use different schemas for storing the subsystem database tables, and the # test/example data, so that they do not accidentally touch the default `doc` # schema. - TESTDRIVE_DATA_SCHEMA = "testdrive-data" TESTDRIVE_EXT_SCHEMA = "testdrive-ext" RESET_TABLES = [ @@ -45,7 +44,7 @@ def cratedb_service(): Provide a CrateDB service instance to the test suite. """ db = CrateDBFixture() - db.start(port=CRATEDB_HTTP_PORT, cmd_opts=CRATEDB_SETTINGS) + db.start(ports={CRATEDB_HTTP_PORT: None}, cmd_opts=CRATEDB_SETTINGS) db.reset(tables=RESET_TABLES) yield db db.stop() From 2c643bdcdd86acf9675de2816debfd0e24d5d09c Mon Sep 17 00:00:00 2001 From: Vitaly Samigullin Date: Thu, 23 Nov 2023 14:38:56 +0100 Subject: [PATCH 08/10] Fix naming for testcontainers wrappers --- cratedb_toolkit/testing/testcontainers/cratedb.py | 9 ++++----- tests/conftest.py | 4 ++-- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/cratedb_toolkit/testing/testcontainers/cratedb.py b/cratedb_toolkit/testing/testcontainers/cratedb.py index 0d5298b..67693b7 100644 --- a/cratedb_toolkit/testing/testcontainers/cratedb.py +++ b/cratedb_toolkit/testing/testcontainers/cratedb.py @@ -152,7 +152,7 @@ def _connect(self): wait_for_logs(self, predicate="o.e.n.Node.*started", timeout=MAX_TRIES) -class CrateDBFixture: +class CrateDBTestAdapter: """ A little helper wrapping Testcontainer's `CrateDBContainer` and CrateDB Toolkit's `DatabaseAdapter`, agnostic of the test framework. @@ -194,9 +194,8 @@ def get_connection_url(self, *args, **kwargs): return self.cratedb.get_connection_url(*args, **kwargs) return None - @property - def http_url(self): + def get_http_url(self, **kwargs): """ - Return a URL for HTTP interface + Return a URL for CrateDB's HTTP endpoint """ - return self.get_connection_url(dialect="http") + return self.get_connection_url(dialect="http", **kwargs) diff --git a/tests/conftest.py b/tests/conftest.py index 181d1a8..efc96b4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -3,7 +3,7 @@ import pytest import responses -from cratedb_toolkit.testing.testcontainers.cratedb import CrateDBFixture +from cratedb_toolkit.testing.testcontainers.cratedb import CrateDBTestAdapter from cratedb_toolkit.util.common import setup_logging # Use different schemas for storing the subsystem database tables, and the @@ -43,7 +43,7 @@ def cratedb_service(): """ Provide a CrateDB service instance to the test suite. """ - db = CrateDBFixture() + db = CrateDBTestAdapter() db.start(ports={CRATEDB_HTTP_PORT: None}, cmd_opts=CRATEDB_SETTINGS) db.reset(tables=RESET_TABLES) yield db From 661370ffb0619e2a4c698c52627c99a1fb726bad Mon Sep 17 00:00:00 2001 From: Vitaly Samigullin Date: Fri, 24 Nov 2023 11:40:44 +0100 Subject: [PATCH 09/10] Remove unused args in CrateDBTestAdapter constructor; fix backward compat --- .../testing/testcontainers/cratedb.py | 31 +++++++++++++------ 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/cratedb_toolkit/testing/testcontainers/cratedb.py b/cratedb_toolkit/testing/testcontainers/cratedb.py index 67693b7..2b5d6ae 100644 --- a/cratedb_toolkit/testing/testcontainers/cratedb.py +++ b/cratedb_toolkit/testing/testcontainers/cratedb.py @@ -70,7 +70,6 @@ def __init__( user: Optional[str] = None, password: Optional[str] = None, dbname: Optional[str] = None, - dialect: str = "crate", cmd_opts: Optional[dict] = None, **kwargs, ) -> None: @@ -79,13 +78,12 @@ def __init__( :param ports: optional dict that maps a port inside the container to a port on the host machine; `None` as a map value generates a random port; Dicts are ordered. By convention the first key-val pair is designated to the HTTP interface. - Example: {4200: None, 5432: 15431} - port 4200 inside the container will be mapped + Example: {4200: None, 5432: 15432} - port 4200 inside the container will be mapped to a random port on the host, internal port 5432 for PSQL interface will be mapped to the 15432 port on the host. - :param user: optional username to access the DB; if None, try respective environment variable - :param password: optional password to access the DB; if None, try respective environment variable - :param dbname: optional database name to access the DB; if None, try respective environment variable - :param dialect: a string with the dialect to generate a DB URI + :param user: optional username to access the DB; if None, try `CRATEDB_USER` environment variable + :param password: optional password to access the DB; if None, try `CRATEDB_PASSWORD` environment variable + :param dbname: optional database name to access the DB; if None, try `CRATEDB_DB` environment variable :param cmd_opts: an optional dict with CLI arguments to be passed to the DB entrypoint inside the container :param kwargs: misc keyword arguments """ @@ -102,7 +100,6 @@ def __init__( self.port_mapping = ports if ports else {4200: None} self.port_to_expose, _ = list(self.port_mapping.items())[0] - self.dialect = dialect @staticmethod def _build_cmd(opts: dict) -> str: @@ -133,11 +130,18 @@ def _configure(self) -> None: self._configure_ports() self._configure_credentials() - def get_connection_url(self, host=None, dialect=None) -> str: + def get_connection_url(self, dialect: str = "crate", host: Optional[str] = None) -> str: + """ + Return a connection URL to the DB + + :param host: optional string + :param dialect: a string with the dialect name to generate a DB URI + :return: string containing a connection URL to te DB + """ # TODO: When using `db_name=self.CRATEDB_DB`: # Connection.__init__() got an unexpected keyword argument 'database' return super()._create_connection_url( - dialect=dialect or self.dialect, + dialect=dialect, username=self.CRATEDB_USER, password=self.CRATEDB_PASSWORD, host=host, @@ -199,3 +203,12 @@ def get_http_url(self, **kwargs): Return a URL for CrateDB's HTTP endpoint """ return self.get_connection_url(dialect="http", **kwargs) + + @property + def http_url(self): + """ + Return a URL for CrateDB's HTTP endpoint. + + Used to stay backward compatible with the downstream code. + """ + return self.get_http_url() From 7ae9a4aa02f1486136ae461f526c321f6ccf963b Mon Sep 17 00:00:00 2001 From: Vitaly Samigullin Date: Fri, 24 Nov 2023 18:59:29 +0100 Subject: [PATCH 10/10] Improve readability --- .../testing/testcontainers/cratedb.py | 4 +-- tests/testing/test_testcontainers.py | 33 +++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 tests/testing/test_testcontainers.py diff --git a/cratedb_toolkit/testing/testcontainers/cratedb.py b/cratedb_toolkit/testing/testcontainers/cratedb.py index 2b5d6ae..183bac0 100644 --- a/cratedb_toolkit/testing/testcontainers/cratedb.py +++ b/cratedb_toolkit/testing/testcontainers/cratedb.py @@ -91,7 +91,7 @@ def __init__( self._name = "testcontainers-cratedb" - cmd_opts = cmd_opts if cmd_opts else {} + cmd_opts = cmd_opts or {} self._command = self._build_cmd({**self.CMD_OPTS, **cmd_opts}) self.CRATEDB_USER = user or self.CRATEDB_USER @@ -110,7 +110,7 @@ def _build_cmd(opts: dict) -> str: for key, val in opts.items(): if isinstance(val, bool): val = str(val).lower() - cmd.append("-C{}={}".format(key, val)) + cmd.append(f"-C{key}={val}") return " ".join(cmd) def _configure_ports(self) -> None: diff --git a/tests/testing/test_testcontainers.py b/tests/testing/test_testcontainers.py new file mode 100644 index 0000000..8b5c4b1 --- /dev/null +++ b/tests/testing/test_testcontainers.py @@ -0,0 +1,33 @@ +import pytest + +from cratedb_toolkit.testing.testcontainers.cratedb import CrateDBContainer + + +@pytest.mark.parametrize( + "opts, expected", + [ + pytest.param( + {"indices.breaker.total.limit": "90%"}, + ( + "-Cdiscovery.type=single-node " + "-Cnode.attr.storage=hot " + "-Cpath.repo=/tmp/snapshots " + "-Cindices.breaker.total.limit=90%" + ), + id="add_cmd_option", + ), + pytest.param( + {"discovery.type": "zen", "indices.breaker.total.limit": "90%"}, + ( + "-Cdiscovery.type=zen " + "-Cnode.attr.storage=hot " + "-Cpath.repo=/tmp/snapshots " + "-Cindices.breaker.total.limit=90%" + ), + id="override_defaults", + ), + ], +) +def test_build_command(opts, expected): + db = CrateDBContainer(cmd_opts=opts) + assert db._command == expected