QA: Migrate type checker from mypy to ty, CodeRabbit improvements 1

Author: amotl

cratedb_toolkit/io/awslambda/kinesis.py

@@ -69,6 +69,10 @@
     message = f"Invalid value for MESSAGE_FORMAT: {MESSAGE_FORMAT}. Use one of: {message_formats}"
     logger.fatal(message)
     sys.exit(22)
+if MESSAGE_FORMAT == "dynamodb" and not CRATEDB_TABLE:
+    message = "CRATEDB_TABLE environment variable is required when MESSAGE_FORMAT is 'dynamodb'"
+    logger.fatal(message)
+    sys.exit(22)
 try:
     column_types = ColumnTypeMapStore.from_json(COLUMN_TYPES)
 except Exception as ex:

cratedb_toolkit/io/mongodb/cdc.py

@@ -62,6 +62,7 @@ def __init__(
         if tm:
             address = CollectionAddress(
                 container=self.mongodb_adapter.database_name,
+                # TODO: Collection name must be permitted to be None?
                 name=t.cast(str, self.mongodb_adapter.collection_name),
             )
             try:

cratedb_toolkit/io/mongodb/copy.py

@@ -4,6 +4,7 @@
 
 import sqlalchemy as sa
 from boltons.urlutils import URL
+from commons_codec.model import SQLOperation
 from commons_codec.transform.mongodb import MongoDBCrateDBConverter, MongoDBFullLoadTranslator
 from tikray.model.collection import CollectionAddress
 from tqdm import tqdm
@@ -13,6 +14,7 @@
 from cratedb_toolkit.io.mongodb.adapter import mongodb_adapter_factory
 from cratedb_toolkit.io.mongodb.transform import TransformationManager
 from cratedb_toolkit.model import DatabaseAddress
+from cratedb_toolkit.util.cli import to_list
 from cratedb_toolkit.util.database import DatabaseAdapter
 
 logger = logging.getLogger(__name__)
@@ -51,6 +53,7 @@ def __init__(
         if tm:
             address = CollectionAddress(
                 container=self.mongodb_adapter.database_name,
+                # TODO: Collection name must be permitted to be None?
                 name=t.cast(str, self.mongodb_adapter.collection_name),
             )
             try:
@@ -88,10 +91,13 @@ def start(self):
             logger.info(f"Target: CrateDB table={self.cratedb_table} count={records_target}")
             progress_bar = tqdm(total=records_in)
 
+            def batch_to_sql_operation(batch: t.List[t.Dict[str, t.Any]]) -> SQLOperation:
+                return self.translator.to_sql(to_list(batch, []))
+
             processor = BulkProcessor(
                 connection=connection,
                 data=self.mongodb_adapter.query(),
-                batch_to_operation=self.translator.to_sql,  # ty: ignore[invalid-argument-type]
+                batch_to_operation=batch_to_sql_operation,
                 progress_bar=progress_bar,
                 on_error=self.on_error,
                 debug=self.debug,

cratedb_toolkit/testing/testcontainers/cratedb.py

@@ -170,17 +170,23 @@ class CrateDBTestAdapter:
     """
 
     def __init__(self, crate_version: str = "nightly", **kwargs):
-        self.cratedb: CrateDBContainer
-        self.database: DatabaseAdapter
+        self.cratedb: Optional[CrateDBContainer] = None
+        self._database: Optional[DatabaseAdapter] = None
         self.image: str = "crate/crate:{}".format(crate_version)
 
+    @property
+    def database(self) -> DatabaseAdapter:
+        if self._database is None:
+            raise ValueError("DatabaseAdapter is not initialized")
+        return self._database
+
     def start(self, **kwargs):
         """
         Start testcontainer, used for tests set up
         """
         self.cratedb = CrateDBContainer(image=self.image, **kwargs)
         self.cratedb.start()
-        self.database = DatabaseAdapter(dburi=self.get_connection_url(), echo=False)
+        self._database = DatabaseAdapter(dburi=self.get_connection_url(), echo=False)
 
     def stop(self):
         """
@@ -193,19 +199,19 @@ def reset(self, tables: Optional[list] = None, schemas: Optional[list] = None):
         """
         Drop tables from the given list, used for tests set up or tear down
         """
-        if not self.database:
+        if not self._database:
             return
 
         if schemas:
             for reset_schema in schemas:
-                self.database.connection.exec_driver_sql(
-                    f"DROP SCHEMA IF EXISTS {self.database.quote_relation_name(reset_schema)} CASCADE;"
+                self._database.connection.exec_driver_sql(
+                    f"DROP SCHEMA IF EXISTS {self._database.quote_relation_name(reset_schema)} CASCADE;"
                 )
 
         if tables:
             for reset_table in tables:
-                self.database.connection.exec_driver_sql(
-                    f"DROP TABLE IF EXISTS {self.database.quote_relation_name(reset_table)};"
+                self._database.connection.exec_driver_sql(
+                    f"DROP TABLE IF EXISTS {self._database.quote_relation_name(reset_table)};"
                 )
 
     def get_connection_url(self, *args, **kwargs):

cratedb_toolkit/testing/testcontainers/mongodb.py

@@ -13,11 +13,13 @@
 import os
 import time
 import typing as t
+from urllib.parse import quote
 
 import pymongo.errors
 from pymongo import MongoClient
 from testcontainers.core.exceptions import ContainerStartException
 from testcontainers.mongodb import MongoDbContainer
+from yarl import URL
 
 from cratedb_toolkit.testing.testcontainers.util import DockerSkippingContainer, KeepaliveContainer
 
@@ -26,7 +28,7 @@ class MongoDbContainerWithKeepalive(DockerSkippingContainer, KeepaliveContainer,
     """
     A Testcontainer for MongoDB with improved configurability.
 
-    It honors the `TC_KEEPALIVE` and `MONGODB_VERSION` environment variables.
+    It honours the `TC_KEEPALIVE` and `MONGODB_VERSION` environment variables.
 
     Defining `TC_KEEPALIVE` will set a signal not to shut down the container
     after running the test cases, in order to speed up subsequent invocations.
@@ -87,6 +89,11 @@ def _create_connection_url(
         dbname: t.Optional[str] = None,
         **kwargs,
     ) -> str:
+        """
+        TODO: Why does this method need to be overwritten?
+        Hint: Probably because of getting rid of credentials forwarding,
+              which is currently intended to reduce configuration complexity.
+        """
         from testcontainers.core.utils import raise_for_deprecated_parameter
 
         if raise_for_deprecated_parameter(kwargs, "db_name", "dbname"):
@@ -96,16 +103,20 @@ def _create_connection_url(
         host = host or self.get_container_host_ip()
         assert port is not None  # noqa: S101
         port = self.get_exposed_port(port)
-        url = f"{dialect}://{host}:{port}"
+        url = URL(f"{dialect}://{host}:{port}")
+        if username:
+            url = url.with_user(username)
+        if password:
+            url = url.with_password(quote(password, safe=" +"))
         if dbname:
-            url = f"{url}/{dbname}"
-        return url
+            url = url.with_path(dbname)
+        return str(url)
 
     def get_connection_url(self) -> str:
         return self._create_connection_url(
             dialect="mongodb",
-            username="admin",
-            password="",
+            username=None,  # ty: ignore[invalid-argument-type]
+            password=None,  # ty: ignore[invalid-argument-type]
             port=self.port,
         )
 

cratedb_toolkit/testing/testcontainers/util.py

@@ -19,6 +19,7 @@
 from docker.errors import DockerException
 from docker.models.containers import Container
 from testcontainers.core.container import DockerContainer
+from testcontainers.core.generic import DbContainer
 
 from cratedb_toolkit.util.data import asbool
 
@@ -176,7 +177,7 @@ class PytestTestcontainerAdapter:
     """
 
     def __init__(self):
-        self.container: DockerContainer
+        self.container: DbContainer
         self.run_setup()
 
     @abstractmethod

cratedb_toolkit/util/client.py

@@ -24,7 +24,7 @@ def __enter__(self):
     def __exit__(self, type, value, tb):  # noqa: A002
         if self.jwt_token:
             self.patcher.stop()
-        return self
+        return False
 
 
 def _mk_crate_client_request(jwt_token: Optional[str] = None):

tests/io/mongodb/test_integration.py

@@ -49,7 +49,7 @@ def setUpClass(cls):
             logger.debug(f"MongoDB server info: {server_info}")
         except pymongo.errors.ServerSelectionTimeoutError as ex:
             if cls.SKIP_IF_NOT_RUNNING:
-                raise cls.skipTest(cls, reason="MongoDB server not running") from ex  # ty: ignore[invalid-argument-type]
+                raise unittest.SkipTest("MongoDB server not running") from ex
             else:  # noqa: RET506
                 raise