From 02d8974ad2b5ee2dbc6fb1e990e8efa5d3b59370 Mon Sep 17 00:00:00 2001 From: Marcin Rudolf Date: Tue, 24 Sep 2024 20:46:29 +0200 Subject: [PATCH 1/2] makes all args to ConnectionStringCredentials optional --- .../specs/connection_string_credentials.py | 12 +++++++++--- tests/common/configuration/test_credentials.py | 6 +++--- tests/common/configuration/test_toml_provider.py | 5 +++-- tests/common/configuration/utils.py | 7 +++++++ 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/dlt/common/configuration/specs/connection_string_credentials.py b/dlt/common/configuration/specs/connection_string_credentials.py index 5d3ec689c4..1da7961ef8 100644 --- a/dlt/common/configuration/specs/connection_string_credentials.py +++ b/dlt/common/configuration/specs/connection_string_credentials.py @@ -10,14 +10,20 @@ @configspec class ConnectionStringCredentials(CredentialsConfiguration): drivername: str = dataclasses.field(default=None, init=False, repr=False, compare=False) - database: str = None + database: Optional[str] = None password: Optional[TSecretValue] = None - username: str = None + username: Optional[str] = None host: Optional[str] = None port: Optional[int] = None query: Optional[Dict[str, Any]] = None - __config_gen_annotations__: ClassVar[List[str]] = ["port", "password", "host"] + __config_gen_annotations__: ClassVar[List[str]] = [ + "database", + "port", + "username", + "password", + "host", + ] def __init__(self, connection_string: Union[str, Dict[str, Any]] = None) -> None: """Initializes the credentials from SQLAlchemy like connection string or from dict holding connection string elements""" diff --git a/tests/common/configuration/test_credentials.py b/tests/common/configuration/test_credentials.py index 1c6319b551..9c09ccacd0 100644 --- a/tests/common/configuration/test_credentials.py +++ b/tests/common/configuration/test_credentials.py @@ -24,7 +24,7 @@ from dlt.destinations.impl.snowflake.configuration import SnowflakeCredentials from tests.utils import TEST_DICT_CONFIG_PROVIDER, preserve_environ from tests.common.utils import json_case_path -from tests.common.configuration.utils import environment +from tests.common.configuration.utils import ConnectionStringCompatCredentials, environment SERVICE_JSON = """ @@ -125,7 +125,7 @@ def test_connection_string_letter_case(environment: Any) -> None: def test_connection_string_resolved_from_native_representation(environment: Any) -> None: destination_dsn = "mysql+pymsql://localhost:5432/dlt_data" - c = ConnectionStringCredentials() + c = ConnectionStringCompatCredentials() c.parse_native_representation(destination_dsn) assert c.is_partial() assert not c.is_resolved() @@ -141,7 +141,7 @@ def test_connection_string_resolved_from_native_representation(environment: Any) assert c.password is None # password must resolve - c = ConnectionStringCredentials() + c = ConnectionStringCompatCredentials() c.parse_native_representation("mysql+pymsql://USER@/dlt_data") # not partial! password is optional assert not c.is_partial() diff --git a/tests/common/configuration/test_toml_provider.py b/tests/common/configuration/test_toml_provider.py index 3b16a930e6..a19aea8796 100644 --- a/tests/common/configuration/test_toml_provider.py +++ b/tests/common/configuration/test_toml_provider.py @@ -32,6 +32,7 @@ from tests.utils import preserve_environ from tests.common.configuration.utils import ( + ConnectionStringCompatCredentials, SecretCredentials, WithCredentialsConfiguration, CoercionTestConfiguration, @@ -150,12 +151,12 @@ def test_secrets_toml_credentials(environment: Any, toml_providers: ConfigProvid with pytest.raises(ConfigFieldMissingException): print(dict(resolve.resolve_configuration(GcpServiceAccountCredentialsWithoutDefaults()))) # also try postgres credentials - c2 = ConnectionStringCredentials() + c2 = ConnectionStringCompatCredentials() c2.update({"drivername": "postgres"}) c2 = resolve.resolve_configuration(c2, sections=("destination", "redshift")) assert c2.database == "destination.redshift.credentials" # bigquery credentials do not match redshift credentials - c3 = ConnectionStringCredentials() + c3 = ConnectionStringCompatCredentials() c3.update({"drivername": "postgres"}) with pytest.raises(ConfigFieldMissingException): resolve.resolve_configuration(c3, sections=("destination", "bigquery")) diff --git a/tests/common/configuration/utils.py b/tests/common/configuration/utils.py index 677ec3d329..c28f93e32b 100644 --- a/tests/common/configuration/utils.py +++ b/tests/common/configuration/utils.py @@ -21,6 +21,7 @@ from dlt.common.configuration.specs import BaseConfiguration, CredentialsConfiguration from dlt.common.configuration.container import Container from dlt.common.configuration.providers import ConfigProvider, EnvironProvider +from dlt.common.configuration.specs.connection_string_credentials import ConnectionStringCredentials from dlt.common.configuration.utils import get_resolved_traces from dlt.common.configuration.specs.config_providers_context import ConfigProvidersContext from dlt.common.typing import TSecretValue, StrAny @@ -78,6 +79,12 @@ class SectionedConfiguration(BaseConfiguration): password: str = None +@configspec +class ConnectionStringCompatCredentials(ConnectionStringCredentials): + database: str = None + username: str = None + + @pytest.fixture(scope="function") def environment() -> Any: saved_environ = environ.copy() From f1defd05de801f46a94274c637b85e8442cd67e3 Mon Sep 17 00:00:00 2001 From: Marcin Rudolf Date: Tue, 24 Sep 2024 20:46:44 +0200 Subject: [PATCH 2/2] adjusts existing derived credentials --- dlt/destinations/impl/duckdb/configuration.py | 5 ----- dlt/destinations/impl/mssql/configuration.py | 2 ++ dlt/destinations/impl/postgres/configuration.py | 2 ++ dlt/destinations/impl/snowflake/configuration.py | 2 +- dlt/destinations/impl/sqlalchemy/configuration.py | 12 +++++++++--- 5 files changed, 14 insertions(+), 9 deletions(-) diff --git a/dlt/destinations/impl/duckdb/configuration.py b/dlt/destinations/impl/duckdb/configuration.py index bc04552078..ec58d66c8b 100644 --- a/dlt/destinations/impl/duckdb/configuration.py +++ b/dlt/destinations/impl/duckdb/configuration.py @@ -25,11 +25,6 @@ @configspec(init=False) class DuckDbBaseCredentials(ConnectionStringCredentials): - password: Optional[TSecretValue] = None - host: Optional[str] = None - port: Optional[int] = None - database: Optional[str] = None - read_only: bool = False # open database read/write def borrow_conn(self, read_only: bool) -> Any: diff --git a/dlt/destinations/impl/mssql/configuration.py b/dlt/destinations/impl/mssql/configuration.py index 5b08546f73..a30b300343 100644 --- a/dlt/destinations/impl/mssql/configuration.py +++ b/dlt/destinations/impl/mssql/configuration.py @@ -14,6 +14,8 @@ @configspec(init=False) class MsSqlCredentials(ConnectionStringCredentials): drivername: Final[str] = dataclasses.field(default="mssql", init=False, repr=False, compare=False) # type: ignore + database: str = None + username: str = None password: TSecretValue = None host: str = None port: int = 1433 diff --git a/dlt/destinations/impl/postgres/configuration.py b/dlt/destinations/impl/postgres/configuration.py index fab398fc21..656d1b3ac1 100644 --- a/dlt/destinations/impl/postgres/configuration.py +++ b/dlt/destinations/impl/postgres/configuration.py @@ -14,6 +14,8 @@ @configspec(init=False) class PostgresCredentials(ConnectionStringCredentials): drivername: Final[str] = dataclasses.field(default="postgresql", init=False, repr=False, compare=False) # type: ignore + database: str = None + username: str = None password: TSecretValue = None host: str = None port: int = 5432 diff --git a/dlt/destinations/impl/snowflake/configuration.py b/dlt/destinations/impl/snowflake/configuration.py index 3fc479f237..de8faa91a6 100644 --- a/dlt/destinations/impl/snowflake/configuration.py +++ b/dlt/destinations/impl/snowflake/configuration.py @@ -56,9 +56,9 @@ def _decode_private_key(private_key: str, password: Optional[str] = None) -> byt @configspec(init=False) class SnowflakeCredentials(ConnectionStringCredentials): drivername: Final[str] = dataclasses.field(default="snowflake", init=False, repr=False, compare=False) # type: ignore[misc] - password: Optional[TSecretStrValue] = None host: str = None database: str = None + username: str = None warehouse: Optional[str] = None role: Optional[str] = None authenticator: Optional[str] = None diff --git a/dlt/destinations/impl/sqlalchemy/configuration.py b/dlt/destinations/impl/sqlalchemy/configuration.py index f99b06a27b..b26c87dfac 100644 --- a/dlt/destinations/impl/sqlalchemy/configuration.py +++ b/dlt/destinations/impl/sqlalchemy/configuration.py @@ -1,4 +1,4 @@ -from typing import TYPE_CHECKING, Optional, Any, Final, Type, Dict, Union +from typing import TYPE_CHECKING, ClassVar, List, Optional, Any, Final, Type, Dict, Union import dataclasses from dlt.common.configuration import configspec @@ -14,8 +14,6 @@ class SqlalchemyCredentials(ConnectionStringCredentials): if TYPE_CHECKING: _engine: Optional["Engine"] = None - username: Optional[str] = None # e.g. sqlite doesn't need username - def __init__( self, connection_string: Optional[Union[str, Dict[str, Any], "Engine"]] = None ) -> None: @@ -49,6 +47,14 @@ def get_dialect(self) -> Optional[Type["Dialect"]]: return type(engine.dialect) return self.to_url().get_dialect() # type: ignore[attr-defined,no-any-return] + __config_gen_annotations__: ClassVar[List[str]] = [ + "database", + "port", + "username", + "password", + "host", + ] + @configspec class SqlalchemyClientConfiguration(DestinationClientDwhConfiguration):