From 6b2f6c4280eea2ea7ae6a36e206fbe99ac0fda99 Mon Sep 17 00:00:00 2001 From: Quinn Batten Date: Mon, 6 Feb 2023 14:48:03 -0500 Subject: [PATCH 01/14] feat: refactor SQLConnector connection handling --- poetry.lock | 3 +- singer_sdk/connectors/sql.py | 61 ++++++++++++++++++++++-------------- 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/poetry.lock b/poetry.lock index f8fdb18b1..ed826f2ef 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1012,6 +1012,7 @@ category = "main" optional = true python-versions = "*" files = [ + {file = "livereload-2.6.3-py2.py3-none-any.whl", hash = "sha256:ad4ac6f53b2d62bb6ce1a5e6e96f1f00976a32348afedcb4b6d68df2a1d346e4"}, {file = "livereload-2.6.3.tar.gz", hash = "sha256:776f2f865e59fde56490a56bcc6773b6917366bce0c267c60ee8aaf1a0959869"}, ] @@ -1596,7 +1597,7 @@ files = [ cffi = ">=1.4.1" [package.extras] -docs = ["sphinx (>=1.6.5)", "sphinx_rtd_theme"] +docs = ["sphinx (>=1.6.5)", "sphinx-rtd-theme"] tests = ["hypothesis (>=3.27.0)", "pytest (>=3.2.1,!=3.3.0)"] [[package]] diff --git a/singer_sdk/connectors/sql.py b/singer_sdk/connectors/sql.py index d3ec81261..9cd6f2d05 100644 --- a/singer_sdk/connectors/sql.py +++ b/singer_sdk/connectors/sql.py @@ -2,10 +2,12 @@ from __future__ import annotations +import functools import logging +from contextlib import contextmanager from datetime import datetime from functools import lru_cache -from typing import Any, Iterable, cast +from typing import Any, Iterable, Iterator, cast import sqlalchemy from sqlalchemy.engine import Engine @@ -46,7 +48,6 @@ def __init__( """ self._config: dict[str, Any] = config or {} self._sqlalchemy_url: str | None = sqlalchemy_url or None - self._connection: sqlalchemy.engine.Connection | None = None @property def config(self) -> dict: @@ -66,8 +67,17 @@ def logger(self) -> logging.Logger: """ return logging.getLogger("sqlconnector") + @contextmanager + def _connect(self) -> Iterator[None]: + with self._engine.connect().execution_options(stream_results=True) as conn: + yield conn + def create_sqlalchemy_connection(self) -> sqlalchemy.engine.Connection: - """Return a new SQLAlchemy connection using the provided config. + """(DEPRECATED) Return a new SQLAlchemy connection using the provided config. + + Do not use the SQLConnector's connection directly. Instead, if you need + to execute something that isn't available on the connector currently, + make a child class and add a method on that connector. By default this will create using the sqlalchemy `stream_results=True` option described here: @@ -81,14 +91,10 @@ def create_sqlalchemy_connection(self) -> sqlalchemy.engine.Connection: Returns: A newly created SQLAlchemy engine object. """ - return ( - self.create_sqlalchemy_engine() - .connect() - .execution_options(stream_results=True) - ) + return self._engine.connect().execution_options(stream_results=True) def create_sqlalchemy_engine(self) -> sqlalchemy.engine.Engine: - """Return a new SQLAlchemy engine using the provided config. + """(DEPRECATED) Return a new SQLAlchemy engine using the provided config. Developers can generally override just one of the following: `sqlalchemy_engine`, sqlalchemy_url`. @@ -96,19 +102,20 @@ def create_sqlalchemy_engine(self) -> sqlalchemy.engine.Engine: Returns: A newly created SQLAlchemy engine object. """ - return sqlalchemy.create_engine(self.sqlalchemy_url, echo=False) + return self._engine @property def connection(self) -> sqlalchemy.engine.Connection: - """Return or set the SQLAlchemy connection object. + """(DEPRECATED) Return or set the SQLAlchemy connection object. + + Do not use the SQLConnector's connection directly. Instead, if you need + to execute something that isn't available on the connector currently, + make a child class and add a method on that connector. Returns: The active SQLAlchemy connection object. """ - if not self._connection: - self._connection = self.create_sqlalchemy_connection() - - return self._connection + return self.create_sqlalchemy_connection() @property def sqlalchemy_url(self) -> str: @@ -249,16 +256,20 @@ def _dialect(self) -> sqlalchemy.engine.Dialect: Returns: The dialect object. """ - return cast(sqlalchemy.engine.Dialect, self.connection.engine.dialect) + return cast(sqlalchemy.engine.Dialect, self._engine.dialect) @property + @functools.lru_cache() def _engine(self) -> sqlalchemy.engine.Engine: - """Return the dialect object. + """Return the engine object. Returns: The dialect object. """ - return cast(sqlalchemy.engine.Engine, self.connection.engine) + return cast( + sqlalchemy.engine.Engine, + sqlalchemy.create_engine(self.sqlalchemy_url, echo=False), + ) def quote(self, name: str) -> str: """Quote a name if it needs quoting, using '.' as a name-part delimiter. @@ -421,7 +432,7 @@ def discover_catalog_entries(self) -> list[dict]: The discovered catalog entries as a list. """ result: list[dict] = [] - engine = self.create_sqlalchemy_engine() + engine = self._engine inspected = sqlalchemy.inspect(engine) for schema_name in self.get_schema_names(engine, inspected): # Iterate through each table and view @@ -562,7 +573,8 @@ def create_schema(self, schema_name: str) -> None: Args: schema_name: The target schema to create. """ - self._engine.execute(sqlalchemy.schema.CreateSchema(schema_name)) + with self._connect() as conn: + conn.execute(sqlalchemy.schema.CreateSchema(schema_name)) def create_empty_table( self, @@ -635,7 +647,8 @@ def _create_empty_column( column_add_ddl = self.get_column_add_ddl( table_name=full_table_name, column_name=column_name, column_type=sql_type ) - self.connection.execute(column_add_ddl) + with self._connect() as conn: + conn.execute(column_add_ddl) def prepare_schema(self, schema_name: str) -> None: """Create the target database schema. @@ -723,7 +736,8 @@ def rename_column(self, full_table_name: str, old_name: str, new_name: str) -> N column_rename_ddl = self.get_column_rename_ddl( table_name=full_table_name, column_name=old_name, new_column_name=new_name ) - self.connection.execute(column_rename_ddl) + with self._connect() as conn: + conn.execute(column_rename_ddl) def merge_sql_types( self, sql_types: list[sqlalchemy.types.TypeEngine] @@ -1027,4 +1041,5 @@ def _adapt_column_type( column_name=column_name, column_type=compatible_sql_type, ) - self.connection.execute(alter_column_ddl) + with self._connect() as conn: + conn.execute(alter_column_ddl) From 0b5d6b0e4b07c93290f68b62e1f807f9e7217ccd Mon Sep 17 00:00:00 2001 From: Quinn Batten Date: Mon, 6 Feb 2023 22:17:33 -0500 Subject: [PATCH 02/14] Fix mypy errors --- singer_sdk/connectors/sql.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/singer_sdk/connectors/sql.py b/singer_sdk/connectors/sql.py index 9cd6f2d05..cc3112e9a 100644 --- a/singer_sdk/connectors/sql.py +++ b/singer_sdk/connectors/sql.py @@ -2,7 +2,6 @@ from __future__ import annotations -import functools import logging from contextlib import contextmanager from datetime import datetime @@ -36,6 +35,7 @@ class SQLConnector: allow_column_alter: bool = False # Whether altering column types is supported. allow_merge_upsert: bool = False # Whether MERGE UPSERT is supported. allow_temp_tables: bool = True # Whether temp tables are supported. + _cached_engine: Engine | None = None def __init__( self, config: dict | None = None, sqlalchemy_url: str | None = None @@ -68,7 +68,7 @@ def logger(self) -> logging.Logger: return logging.getLogger("sqlconnector") @contextmanager - def _connect(self) -> Iterator[None]: + def _connect(self) -> Iterator[sqlalchemy.engine.Connection]: with self._engine.connect().execution_options(stream_results=True) as conn: yield conn @@ -93,7 +93,7 @@ def create_sqlalchemy_connection(self) -> sqlalchemy.engine.Connection: """ return self._engine.connect().execution_options(stream_results=True) - def create_sqlalchemy_engine(self) -> sqlalchemy.engine.Engine: + def create_sqlalchemy_engine(self) -> Engine: """(DEPRECATED) Return a new SQLAlchemy engine using the provided config. Developers can generally override just one of the following: @@ -259,17 +259,17 @@ def _dialect(self) -> sqlalchemy.engine.Dialect: return cast(sqlalchemy.engine.Dialect, self._engine.dialect) @property - @functools.lru_cache() - def _engine(self) -> sqlalchemy.engine.Engine: + def _engine(self) -> Engine: """Return the engine object. Returns: The dialect object. """ - return cast( - sqlalchemy.engine.Engine, - sqlalchemy.create_engine(self.sqlalchemy_url, echo=False), - ) + if not self._cached_engine: + self._cached_engine = sqlalchemy.create_engine( + self.sqlalchemy_url, echo=False + ) + return cast(Engine, self._cached_engine) def quote(self, name: str) -> str: """Quote a name if it needs quoting, using '.' as a name-part delimiter. From d7dbfcbd8f2ef2a05a27cffab05d6f2c1f6ec35a Mon Sep 17 00:00:00 2001 From: Quinn Batten Date: Tue, 7 Feb 2023 10:39:35 -0500 Subject: [PATCH 03/14] Apply suggestions from code review Add deprecation warnings. Co-authored-by: Ken Payne --- singer_sdk/connectors/sql.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/singer_sdk/connectors/sql.py b/singer_sdk/connectors/sql.py index cc3112e9a..1d7da1f9f 100644 --- a/singer_sdk/connectors/sql.py +++ b/singer_sdk/connectors/sql.py @@ -3,6 +3,7 @@ from __future__ import annotations import logging +import warnings from contextlib import contextmanager from datetime import datetime from functools import lru_cache @@ -91,6 +92,13 @@ def create_sqlalchemy_connection(self) -> sqlalchemy.engine.Connection: Returns: A newly created SQLAlchemy engine object. """ + warnings.warn( + "`create_sqlalchemy_connection` is deprecated. " + "If you need to execute something that isn't available " + "on the connector currently, make a child class and " + "add your required method on that connector.", + DeprecationWarning + ) return self._engine.connect().execution_options(stream_results=True) def create_sqlalchemy_engine(self) -> Engine: @@ -102,6 +110,10 @@ def create_sqlalchemy_engine(self) -> Engine: Returns: A newly created SQLAlchemy engine object. """ + warnings.warn( + "`create_sqlalchemy_engine` is deprecated. Override `sqlalchemy_engine` or sqlalchemy_url` instead.", + DeprecationWarning + ) return self._engine @property @@ -115,6 +127,12 @@ def connection(self) -> sqlalchemy.engine.Connection: Returns: The active SQLAlchemy connection object. """ + warnings.warn( + "`connection` is deprecated. If you need to execute something " + "that isn't available on the connector currently, make a child " + "class and add your required method on that connector.", + DeprecationWarning + ) return self.create_sqlalchemy_connection() @property From 6e3a495eb16edf5442eb256ca46e5bfbeddf467a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 7 Feb 2023 15:39:58 +0000 Subject: [PATCH 04/14] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- singer_sdk/connectors/sql.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/singer_sdk/connectors/sql.py b/singer_sdk/connectors/sql.py index 1d7da1f9f..3a68cec18 100644 --- a/singer_sdk/connectors/sql.py +++ b/singer_sdk/connectors/sql.py @@ -97,7 +97,7 @@ def create_sqlalchemy_connection(self) -> sqlalchemy.engine.Connection: "If you need to execute something that isn't available " "on the connector currently, make a child class and " "add your required method on that connector.", - DeprecationWarning + DeprecationWarning, ) return self._engine.connect().execution_options(stream_results=True) @@ -112,7 +112,7 @@ def create_sqlalchemy_engine(self) -> Engine: """ warnings.warn( "`create_sqlalchemy_engine` is deprecated. Override `sqlalchemy_engine` or sqlalchemy_url` instead.", - DeprecationWarning + DeprecationWarning, ) return self._engine @@ -131,7 +131,7 @@ def connection(self) -> sqlalchemy.engine.Connection: "`connection` is deprecated. If you need to execute something " "that isn't available on the connector currently, make a child " "class and add your required method on that connector.", - DeprecationWarning + DeprecationWarning, ) return self.create_sqlalchemy_connection() From bb136040bc9fb030ba96e26df614f66a544a7ea1 Mon Sep 17 00:00:00 2001 From: Quinn Batten Date: Tue, 7 Feb 2023 11:32:21 -0500 Subject: [PATCH 05/14] Small fixes --- poetry.lock | 6 +++--- singer_sdk/connectors/sql.py | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/poetry.lock b/poetry.lock index 3139e52c2..9d9a4c3c2 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2244,7 +2244,7 @@ importlib-metadata = {version = "*", markers = "python_version < \"3.8\""} [package.extras] aiomysql = ["aiomysql", "greenlet (!=0.4.17)"] -aiosqlite = ["aiosqlite", "greenlet (!=0.4.17)", "typing-extensions (!=3.10.0.1)"] +aiosqlite = ["aiosqlite", "greenlet (!=0.4.17)", "typing_extensions (!=3.10.0.1)"] asyncio = ["greenlet (!=0.4.17)"] asyncmy = ["asyncmy (>=0.2.3,!=0.2.4)", "greenlet (!=0.4.17)"] mariadb-connector = ["mariadb (>=1.0.1,!=1.1.2)"] @@ -2254,14 +2254,14 @@ mssql-pyodbc = ["pyodbc"] mypy = ["mypy (>=0.910)", "sqlalchemy2-stubs"] mysql = ["mysqlclient (>=1.4.0)", "mysqlclient (>=1.4.0,<2)"] mysql-connector = ["mysql-connector-python"] -oracle = ["cx-oracle (>=7)", "cx-oracle (>=7,<8)"] +oracle = ["cx_oracle (>=7)", "cx_oracle (>=7,<8)"] postgresql = ["psycopg2 (>=2.7)"] postgresql-asyncpg = ["asyncpg", "greenlet (!=0.4.17)"] postgresql-pg8000 = ["pg8000 (>=1.16.6,!=1.29.0)"] postgresql-psycopg2binary = ["psycopg2-binary"] postgresql-psycopg2cffi = ["psycopg2cffi"] pymysql = ["pymysql", "pymysql (<1)"] -sqlcipher = ["sqlcipher3-binary"] +sqlcipher = ["sqlcipher3_binary"] [[package]] name = "sqlalchemy2-stubs" diff --git a/singer_sdk/connectors/sql.py b/singer_sdk/connectors/sql.py index 3a68cec18..2097d6792 100644 --- a/singer_sdk/connectors/sql.py +++ b/singer_sdk/connectors/sql.py @@ -111,7 +111,8 @@ def create_sqlalchemy_engine(self) -> Engine: A newly created SQLAlchemy engine object. """ warnings.warn( - "`create_sqlalchemy_engine` is deprecated. Override `sqlalchemy_engine` or sqlalchemy_url` instead.", + "`create_sqlalchemy_engine` is deprecated. Override" + "`_engine` or sqlalchemy_url` instead.", DeprecationWarning, ) return self._engine @@ -281,7 +282,7 @@ def _engine(self) -> Engine: """Return the engine object. Returns: - The dialect object. + The SQLAlchemy Engine that's attached to this SQLConnector instance. """ if not self._cached_engine: self._cached_engine = sqlalchemy.create_engine( From 086d9c67daabf45359a415fb15dfb15d65bd24aa Mon Sep 17 00:00:00 2001 From: Quinn Batten Date: Tue, 7 Feb 2023 13:56:18 -0500 Subject: [PATCH 06/14] Improve text of warnings, per code review Co-authored-by: Edgar R. M. --- singer_sdk/connectors/sql.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/singer_sdk/connectors/sql.py b/singer_sdk/connectors/sql.py index 2097d6792..69d8db228 100644 --- a/singer_sdk/connectors/sql.py +++ b/singer_sdk/connectors/sql.py @@ -93,7 +93,7 @@ def create_sqlalchemy_connection(self) -> sqlalchemy.engine.Connection: A newly created SQLAlchemy engine object. """ warnings.warn( - "`create_sqlalchemy_connection` is deprecated. " + "`SQLConnector.create_sqlalchemy_connection` is deprecated. " "If you need to execute something that isn't available " "on the connector currently, make a child class and " "add your required method on that connector.", @@ -111,7 +111,7 @@ def create_sqlalchemy_engine(self) -> Engine: A newly created SQLAlchemy engine object. """ warnings.warn( - "`create_sqlalchemy_engine` is deprecated. Override" + "`SQLConnector.create_sqlalchemy_engine` is deprecated. Override" "`_engine` or sqlalchemy_url` instead.", DeprecationWarning, ) @@ -129,7 +129,7 @@ def connection(self) -> sqlalchemy.engine.Connection: The active SQLAlchemy connection object. """ warnings.warn( - "`connection` is deprecated. If you need to execute something " + "`SQLConnector.connection` is deprecated. If you need to execute something " "that isn't available on the connector currently, make a child " "class and add your required method on that connector.", DeprecationWarning, From 21a63a35f868361f4718add0d4b5437d499fbde8 Mon Sep 17 00:00:00 2001 From: Quinn Batten Date: Wed, 8 Feb 2023 18:26:16 -0500 Subject: [PATCH 07/14] Break out engine creation logic into its own method --- singer_sdk/connectors/sql.py | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/singer_sdk/connectors/sql.py b/singer_sdk/connectors/sql.py index 69d8db228..500d6e585 100644 --- a/singer_sdk/connectors/sql.py +++ b/singer_sdk/connectors/sql.py @@ -279,17 +279,31 @@ def _dialect(self) -> sqlalchemy.engine.Dialect: @property def _engine(self) -> Engine: - """Return the engine object. + """Return the engine object. This is the correct way to access the + Connector's engine, if needed (e.g. to inspect tables). Returns: The SQLAlchemy Engine that's attached to this SQLConnector instance. """ if not self._cached_engine: - self._cached_engine = sqlalchemy.create_engine( - self.sqlalchemy_url, echo=False - ) + self._cached_engine = self.create_engine() return cast(Engine, self._cached_engine) + def create_engine(self) -> Engine: + """Creates and returns a new engine. NOTE: Do not call this method. The + only place that this method should be called is inside the self._engine + method. If you'd like to access the engine on a connector, use + self._engine. + + This method exists solely so that tap/target developers can override it + on their subclass of SQLConnector to perform custom engine creation + logic. + + Returns: + A new SQLAlchemy Engine. + """ + return sqlalchemy.create_engine(self.sqlalchemy_url, echo=False) + def quote(self, name: str) -> str: """Quote a name if it needs quoting, using '.' as a name-part delimiter. From 133e4cf5d427b5410e0665f5d803d4648f40152e Mon Sep 17 00:00:00 2001 From: Quinn Batten Date: Wed, 8 Feb 2023 18:37:15 -0500 Subject: [PATCH 08/14] Fix pre-commit --- poetry.lock | 50 +++++++++++++++++++++++++++++++++++- singer_sdk/connectors/sql.py | 15 ++++++----- 2 files changed, 58 insertions(+), 7 deletions(-) diff --git a/poetry.lock b/poetry.lock index 6989d7ab8..80cfdbe78 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1049,6 +1049,7 @@ category = "main" optional = true python-versions = "*" files = [ + {file = "livereload-2.6.3-py2.py3-none-any.whl", hash = "sha256:ad4ac6f53b2d62bb6ce1a5e6e96f1f00976a32348afedcb4b6d68df2a1d346e4"}, {file = "livereload-2.6.3.tar.gz", hash = "sha256:776f2f865e59fde56490a56bcc6773b6917366bce0c267c60ee8aaf1a0959869"}, ] @@ -1629,7 +1630,7 @@ files = [ cffi = ">=1.4.1" [package.extras] -docs = ["sphinx (>=1.6.5)", "sphinx_rtd_theme"] +docs = ["sphinx (>=1.6.5)", "sphinx-rtd-theme"] tests = ["hypothesis (>=3.27.0)", "pytest (>=3.2.1,!=3.3.0)"] [[package]] @@ -1811,6 +1812,13 @@ files = [ {file = "PyYAML-6.0-cp310-cp310-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl", hash = "sha256:f84fbc98b019fef2ee9a1cb3ce93e3187a6df0b2538a651bfb890254ba9f90b5"}, {file = "PyYAML-6.0-cp310-cp310-win32.whl", hash = "sha256:2cd5df3de48857ed0544b34e2d40e9fac445930039f3cfe4bcc592a1f836d513"}, {file = "PyYAML-6.0-cp310-cp310-win_amd64.whl", hash = "sha256:daf496c58a8c52083df09b80c860005194014c3698698d1a57cbcfa182142a3a"}, + {file = "PyYAML-6.0-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:d4b0ba9512519522b118090257be113b9468d804b19d63c71dbcf4a48fa32358"}, + {file = "PyYAML-6.0-cp311-cp311-macosx_11_0_arm64.whl", hash = "sha256:81957921f441d50af23654aa6c5e5eaf9b06aba7f0a19c18a538dc7ef291c5a1"}, + {file = "PyYAML-6.0-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:afa17f5bc4d1b10afd4466fd3a44dc0e245382deca5b3c353d8b757f9e3ecb8d"}, + {file = "PyYAML-6.0-cp311-cp311-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:dbad0e9d368bb989f4515da330b88a057617d16b6a8245084f1b05400f24609f"}, + {file = "PyYAML-6.0-cp311-cp311-manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:432557aa2c09802be39460360ddffd48156e30721f5e8d917f01d31694216782"}, + {file = "PyYAML-6.0-cp311-cp311-win32.whl", hash = "sha256:bfaef573a63ba8923503d27530362590ff4f576c626d86a9fed95822a8255fd7"}, + {file = "PyYAML-6.0-cp311-cp311-win_amd64.whl", hash = "sha256:01b45c0191e6d66c470b6cf1b9531a771a83c1c4208272ead47a3ae4f2f603bf"}, {file = "PyYAML-6.0-cp36-cp36m-macosx_10_9_x86_64.whl", hash = "sha256:897b80890765f037df3403d22bab41627ca8811ae55e9a722fd0392850ec4d86"}, {file = "PyYAML-6.0-cp36-cp36m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:50602afada6d6cbfad699b0c7bb50d5ccffa7e46a3d738092afddc1f9758427f"}, {file = "PyYAML-6.0-cp36-cp36m-manylinux_2_17_s390x.manylinux2014_s390x.whl", hash = "sha256:48c346915c114f5fdb3ead70312bd042a953a8ce5c7106d5bfb1a5254e47da92"}, @@ -2246,6 +2254,46 @@ category = "main" optional = false python-versions = "!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*,!=3.4.*,!=3.5.*,>=2.7" files = [ + {file = "SQLAlchemy-1.4.46-cp27-cp27m-macosx_10_14_x86_64.whl", hash = "sha256:7001f16a9a8e06488c3c7154827c48455d1c1507d7228d43e781afbc8ceccf6d"}, + {file = "SQLAlchemy-1.4.46-cp27-cp27m-manylinux_2_5_x86_64.manylinux1_x86_64.whl", hash = "sha256:c7a46639ba058d320c9f53a81db38119a74b8a7a1884df44d09fbe807d028aaf"}, + {file = "SQLAlchemy-1.4.46-cp27-cp27m-win32.whl", hash = "sha256:c04144a24103135ea0315d459431ac196fe96f55d3213bfd6d39d0247775c854"}, + {file = "SQLAlchemy-1.4.46-cp27-cp27m-win_amd64.whl", hash = "sha256:7b81b1030c42b003fc10ddd17825571603117f848814a344d305262d370e7c34"}, + {file = "SQLAlchemy-1.4.46-cp27-cp27mu-manylinux_2_5_x86_64.manylinux1_x86_64.whl", hash = "sha256:939f9a018d2ad04036746e15d119c0428b1e557470361aa798e6e7d7f5875be0"}, + {file = "SQLAlchemy-1.4.46-cp310-cp310-macosx_11_0_x86_64.whl", hash = "sha256:b7f4b6aa6e87991ec7ce0e769689a977776db6704947e562102431474799a857"}, + {file = "SQLAlchemy-1.4.46-cp310-cp310-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:5dbf17ac9a61e7a3f1c7ca47237aac93cabd7f08ad92ac5b96d6f8dea4287fc1"}, + {file = "SQLAlchemy-1.4.46-cp310-cp310-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl", hash = "sha256:7f8267682eb41a0584cf66d8a697fef64b53281d01c93a503e1344197f2e01fe"}, + {file = "SQLAlchemy-1.4.46-cp310-cp310-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:64cb0ad8a190bc22d2112001cfecdec45baffdf41871de777239da6a28ed74b6"}, + {file = "SQLAlchemy-1.4.46-cp310-cp310-win32.whl", hash = "sha256:5f752676fc126edc1c4af0ec2e4d2adca48ddfae5de46bb40adbd3f903eb2120"}, + {file = "SQLAlchemy-1.4.46-cp310-cp310-win_amd64.whl", hash = "sha256:31de1e2c45e67a5ec1ecca6ec26aefc299dd5151e355eb5199cd9516b57340be"}, + {file = "SQLAlchemy-1.4.46-cp311-cp311-macosx_10_9_universal2.whl", hash = "sha256:d68e1762997bfebf9e5cf2a9fd0bcf9ca2fdd8136ce7b24bbd3bbfa4328f3e4a"}, + {file = "SQLAlchemy-1.4.46-cp311-cp311-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:4d112b0f3c1bc5ff70554a97344625ef621c1bfe02a73c5d97cac91f8cd7a41e"}, + {file = "SQLAlchemy-1.4.46-cp311-cp311-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:69fac0a7054d86b997af12dc23f581cf0b25fb1c7d1fed43257dee3af32d3d6d"}, + {file = "SQLAlchemy-1.4.46-cp311-cp311-win32.whl", hash = "sha256:887865924c3d6e9a473dc82b70977395301533b3030d0f020c38fd9eba5419f2"}, + {file = "SQLAlchemy-1.4.46-cp311-cp311-win_amd64.whl", hash = "sha256:984ee13543a346324319a1fb72b698e521506f6f22dc37d7752a329e9cd00a32"}, + {file = "SQLAlchemy-1.4.46-cp36-cp36m-macosx_10_14_x86_64.whl", hash = "sha256:9167d4227b56591a4cc5524f1b79ccd7ea994f36e4c648ab42ca995d28ebbb96"}, + {file = "SQLAlchemy-1.4.46-cp36-cp36m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:d61e9ecc849d8d44d7f80894ecff4abe347136e9d926560b818f6243409f3c86"}, + {file = "SQLAlchemy-1.4.46-cp36-cp36m-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl", hash = "sha256:3ec187acf85984263299a3f15c34a6c0671f83565d86d10f43ace49881a82718"}, + {file = "SQLAlchemy-1.4.46-cp36-cp36m-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:9883f5fae4fd8e3f875adc2add69f8b945625811689a6c65866a35ee9c0aea23"}, + {file = "SQLAlchemy-1.4.46-cp36-cp36m-win32.whl", hash = "sha256:535377e9b10aff5a045e3d9ada8a62d02058b422c0504ebdcf07930599890eb0"}, + {file = "SQLAlchemy-1.4.46-cp36-cp36m-win_amd64.whl", hash = "sha256:18cafdb27834fa03569d29f571df7115812a0e59fd6a3a03ccb0d33678ec8420"}, + {file = "SQLAlchemy-1.4.46-cp37-cp37m-macosx_10_15_x86_64.whl", hash = "sha256:a1ad90c97029cc3ab4ffd57443a20fac21d2ec3c89532b084b073b3feb5abff3"}, + {file = "SQLAlchemy-1.4.46-cp37-cp37m-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:4847f4b1d822754e35707db913396a29d874ee77b9c3c3ef3f04d5a9a6209618"}, + {file = "SQLAlchemy-1.4.46-cp37-cp37m-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl", hash = "sha256:c5a99282848b6cae0056b85da17392a26b2d39178394fc25700bcf967e06e97a"}, + {file = "SQLAlchemy-1.4.46-cp37-cp37m-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:d4b1cc7835b39835c75cf7c20c926b42e97d074147c902a9ebb7cf2c840dc4e2"}, + {file = "SQLAlchemy-1.4.46-cp37-cp37m-win32.whl", hash = "sha256:c522e496f9b9b70296a7675272ec21937ccfc15da664b74b9f58d98a641ce1b6"}, + {file = "SQLAlchemy-1.4.46-cp37-cp37m-win_amd64.whl", hash = "sha256:ae067ab639fa499f67ded52f5bc8e084f045d10b5ac7bb928ae4ca2b6c0429a5"}, + {file = "SQLAlchemy-1.4.46-cp38-cp38-macosx_10_15_x86_64.whl", hash = "sha256:e3c1808008124850115a3f7e793a975cfa5c8a26ceeeb9ff9cbb4485cac556df"}, + {file = "SQLAlchemy-1.4.46-cp38-cp38-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:d4d164df3d83d204c69f840da30b292ac7dc54285096c6171245b8d7807185aa"}, + {file = "SQLAlchemy-1.4.46-cp38-cp38-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl", hash = "sha256:b33ffbdbbf5446cf36cd4cc530c9d9905d3c2fe56ed09e25c22c850cdb9fac92"}, + {file = "SQLAlchemy-1.4.46-cp38-cp38-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:3d94682732d1a0def5672471ba42a29ff5e21bb0aae0afa00bb10796fc1e28dd"}, + {file = "SQLAlchemy-1.4.46-cp38-cp38-win32.whl", hash = "sha256:f8cb80fe8d14307e4124f6fad64dfd87ab749c9d275f82b8b4ec84c84ecebdbe"}, + {file = "SQLAlchemy-1.4.46-cp38-cp38-win_amd64.whl", hash = "sha256:07e48cbcdda6b8bc7a59d6728bd3f5f574ffe03f2c9fb384239f3789c2d95c2e"}, + {file = "SQLAlchemy-1.4.46-cp39-cp39-macosx_11_0_x86_64.whl", hash = "sha256:1b1e5e96e2789d89f023d080bee432e2fef64d95857969e70d3cadec80bd26f0"}, + {file = "SQLAlchemy-1.4.46-cp39-cp39-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:a3714e5b33226131ac0da60d18995a102a17dddd42368b7bdd206737297823ad"}, + {file = "SQLAlchemy-1.4.46-cp39-cp39-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_12_x86_64.manylinux2010_x86_64.whl", hash = "sha256:955162ad1a931fe416eded6bb144ba891ccbf9b2e49dc7ded39274dd9c5affc5"}, + {file = "SQLAlchemy-1.4.46-cp39-cp39-manylinux_2_5_x86_64.manylinux1_x86_64.manylinux_2_17_x86_64.manylinux2014_x86_64.whl", hash = "sha256:b6e4cb5c63f705c9d546a054c60d326cbde7421421e2d2565ce3e2eee4e1a01f"}, + {file = "SQLAlchemy-1.4.46-cp39-cp39-win32.whl", hash = "sha256:51e1ba2884c6a2b8e19109dc08c71c49530006c1084156ecadfaadf5f9b8b053"}, + {file = "SQLAlchemy-1.4.46-cp39-cp39-win_amd64.whl", hash = "sha256:315676344e3558f1f80d02535f410e80ea4e8fddba31ec78fe390eff5fb8f466"}, {file = "SQLAlchemy-1.4.46.tar.gz", hash = "sha256:6913b8247d8a292ef8315162a51931e2b40ce91681f1b6f18f697045200c4a30"}, ] diff --git a/singer_sdk/connectors/sql.py b/singer_sdk/connectors/sql.py index 500d6e585..437e97134 100644 --- a/singer_sdk/connectors/sql.py +++ b/singer_sdk/connectors/sql.py @@ -279,8 +279,10 @@ def _dialect(self) -> sqlalchemy.engine.Dialect: @property def _engine(self) -> Engine: - """Return the engine object. This is the correct way to access the - Connector's engine, if needed (e.g. to inspect tables). + """Return the engine object. + + This is the correct way to access the Connector's engine, if needed + (e.g. to inspect tables). Returns: The SQLAlchemy Engine that's attached to this SQLConnector instance. @@ -290,10 +292,11 @@ def _engine(self) -> Engine: return cast(Engine, self._cached_engine) def create_engine(self) -> Engine: - """Creates and returns a new engine. NOTE: Do not call this method. The - only place that this method should be called is inside the self._engine - method. If you'd like to access the engine on a connector, use - self._engine. + """Creates and returns a new engine. Do not call outside of _engine. + + NOTE: Do not call this method. The only place that this method should + be called is inside the self._engine method. If you'd like to access + the engine on a connector, use self._engine. This method exists solely so that tap/target developers can override it on their subclass of SQLConnector to perform custom engine creation From 35a70f9b99b08eab9b67edc2b49ff5ac3154b13d Mon Sep 17 00:00:00 2001 From: Quinn Batten Date: Wed, 8 Feb 2023 21:09:28 -0500 Subject: [PATCH 09/14] Add some tests --- tests/core/test_connector_sql.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/core/test_connector_sql.py b/tests/core/test_connector_sql.py index 8ad7bf9a1..76fce261c 100644 --- a/tests/core/test_connector_sql.py +++ b/tests/core/test_connector_sql.py @@ -14,7 +14,7 @@ class TestConnectorSQL: @pytest.fixture() def connector(self): - return SQLConnector() + return SQLConnector(config={"sqlalchemy_url": "sqlite:///tmp.db"}) @pytest.mark.parametrize( "method_name,kwargs,context,unrendered_statement,rendered_statement", @@ -130,3 +130,14 @@ def test_update_collation_non_text_type(self): assert not hasattr(compatible_type, "collation") # Check that we get the same type we put in assert str(compatible_type) == "INTEGER" + + def test_create_engine_returns_new_engine(self, connector): + engine1 = connector.create_engine() + engine2 = connector.create_engine() + assert engine1 is not engine2 + + def test_engine_creates_and_returns_cached_engine(self, connector): + assert not connector._cached_engine + engine1 = connector._engine + engine2 = connector._cached_engine + assert engine1 is engine2 From 160a7108e4052a39c51db6678291506216e3c811 Mon Sep 17 00:00:00 2001 From: Quinn Batten Date: Wed, 8 Feb 2023 21:59:39 -0500 Subject: [PATCH 10/14] Add more tests --- tests/core/test_connector_sql.py | 8 ++++++++ tmp.db | 0 2 files changed, 8 insertions(+) create mode 100644 tmp.db diff --git a/tests/core/test_connector_sql.py b/tests/core/test_connector_sql.py index 76fce261c..adf590828 100644 --- a/tests/core/test_connector_sql.py +++ b/tests/core/test_connector_sql.py @@ -141,3 +141,11 @@ def test_engine_creates_and_returns_cached_engine(self, connector): engine1 = connector._engine engine2 = connector._cached_engine assert engine1 is engine2 + + def test_deprecated_functions_raise(self, connector): + with pytest.deprecated_call(): + connector.create_sqlalchemy_engine() + with pytest.deprecated_call(): + connector.create_sqlalchemy_connection() + with pytest.deprecated_call(): + connector.connection diff --git a/tmp.db b/tmp.db new file mode 100644 index 000000000..e69de29bb From 3fc2cd6ae4eb3d7bcc5ae6479b5618bd3599d4bb Mon Sep 17 00:00:00 2001 From: Quinn Batten Date: Thu, 9 Feb 2023 17:26:24 -0500 Subject: [PATCH 11/14] Add more tests, pt2 --- tests/core/test_connector_sql.py | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/tests/core/test_connector_sql.py b/tests/core/test_connector_sql.py index adf590828..16f7050e3 100644 --- a/tests/core/test_connector_sql.py +++ b/tests/core/test_connector_sql.py @@ -1,3 +1,5 @@ +from unittest import mock + import pytest import sqlalchemy from sqlalchemy.dialects import sqlite @@ -142,10 +144,27 @@ def test_engine_creates_and_returns_cached_engine(self, connector): engine2 = connector._cached_engine assert engine1 is engine2 - def test_deprecated_functions_raise(self, connector): + def test_deprecated_functions_warn(self, connector): with pytest.deprecated_call(): connector.create_sqlalchemy_engine() with pytest.deprecated_call(): connector.create_sqlalchemy_connection() with pytest.deprecated_call(): connector.connection + + def test_connect_calls_engine(self, connector): + with mock.patch.object(SQLConnector, "_engine") as mock_engine: + with connector._connect() as conn: + mock_engine.connect.assert_called_once() + + def test_connect_calls_engine(self, connector): + attached_engine = connector._engine + with mock.patch.object(attached_engine, "connect") as mock_conn: + with connector._connect() as conn: + mock_conn.assert_called_once() + + def test_connect_raises_on_operational_failure(self): + connector = SQLConnector(config={"sqlalchemy_url": "sqlite:///:memory:"}) + with pytest.raises(sqlalchemy.exc.OperationalError) as e: + with connector._connect() as conn: + conn.execute("SELECT * FROM fake_table") From c4642cb780b64e15805fdd9b913f7c6fa7be1a23 Mon Sep 17 00:00:00 2001 From: Quinn Batten Date: Thu, 9 Feb 2023 18:39:30 -0500 Subject: [PATCH 12/14] Add more tests, pt3 --- tests/core/test_connector_sql.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/core/test_connector_sql.py b/tests/core/test_connector_sql.py index 16f7050e3..0b51ab720 100644 --- a/tests/core/test_connector_sql.py +++ b/tests/core/test_connector_sql.py @@ -5,6 +5,7 @@ from sqlalchemy.dialects import sqlite from singer_sdk.connectors import SQLConnector +from singer_sdk.exceptions import ConfigValidationError def stringify(in_dict): @@ -164,7 +165,29 @@ def test_connect_calls_engine(self, connector): mock_conn.assert_called_once() def test_connect_raises_on_operational_failure(self): + # Using in-memory db here so we aren't writing anything to disk connector = SQLConnector(config={"sqlalchemy_url": "sqlite:///:memory:"}) with pytest.raises(sqlalchemy.exc.OperationalError) as e: with connector._connect() as conn: conn.execute("SELECT * FROM fake_table") + + def test_rename_column_uses_connect_correctly(self, connector): + attached_engine = connector._engine + # Ends up using the attached engine + with mock.patch.object(attached_engine, "connect") as mock_conn: + connector.rename_column("fake_table", "old_name", "new_name") + mock_conn.assert_called_once() + # Uses the _connect method + with mock.patch.object(connector, "_connect") as mock_connect_method: + connector.rename_column("fake_table", "old_name", "new_name") + mock_connect_method.assert_called_once() + + def test_get_slalchemy_url_raises_if_not_in_config(self, connector): + with pytest.raises(ConfigValidationError): + connector.get_sqlalchemy_url({}) + + def test_dialect_uses_engine(self, connector): + attached_engine = connector._engine + with mock.patch.object(attached_engine, "dialect") as mock_dialect: + res = connector._dialect + assert res == attached_engine.dialect From b12f3fbd3d04eba7166d91edcd031d33db7d3b67 Mon Sep 17 00:00:00 2001 From: Quinn Batten Date: Thu, 9 Feb 2023 19:35:21 -0500 Subject: [PATCH 13/14] Tests use in-mem db --- tests/core/test_connector_sql.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/core/test_connector_sql.py b/tests/core/test_connector_sql.py index 0b51ab720..89a7d46a2 100644 --- a/tests/core/test_connector_sql.py +++ b/tests/core/test_connector_sql.py @@ -17,7 +17,7 @@ class TestConnectorSQL: @pytest.fixture() def connector(self): - return SQLConnector(config={"sqlalchemy_url": "sqlite:///tmp.db"}) + return SQLConnector(config={"sqlalchemy_url": "sqlite:///"}) @pytest.mark.parametrize( "method_name,kwargs,context,unrendered_statement,rendered_statement", @@ -164,9 +164,7 @@ def test_connect_calls_engine(self, connector): with connector._connect() as conn: mock_conn.assert_called_once() - def test_connect_raises_on_operational_failure(self): - # Using in-memory db here so we aren't writing anything to disk - connector = SQLConnector(config={"sqlalchemy_url": "sqlite:///:memory:"}) + def test_connect_raises_on_operational_failure(self, connector): with pytest.raises(sqlalchemy.exc.OperationalError) as e: with connector._connect() as conn: conn.execute("SELECT * FROM fake_table") From 5208971519f3de0e09836fc804dfb21c05556bce Mon Sep 17 00:00:00 2001 From: "Edgar R. M" Date: Fri, 10 Feb 2023 10:54:05 -0600 Subject: [PATCH 14/14] Delete tmp.db --- tmp.db | 0 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 tmp.db diff --git a/tmp.db b/tmp.db deleted file mode 100644 index e69de29bb..000000000