Skip to content

Commit

Permalink
Fix/1858 make all connection string credentials optional (#1867)
Browse files Browse the repository at this point in the history
* makes all args to ConnectionStringCredentials optional

* adjusts existing derived credentials
  • Loading branch information
rudolfix authored Sep 25, 2024
1 parent 27c110a commit a938752
Show file tree
Hide file tree
Showing 9 changed files with 36 additions and 17 deletions.
12 changes: 9 additions & 3 deletions dlt/common/configuration/specs/connection_string_credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"""
Expand Down
5 changes: 0 additions & 5 deletions dlt/destinations/impl/duckdb/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 2 additions & 0 deletions dlt/destinations/impl/mssql/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions dlt/destinations/impl/postgres/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion dlt/destinations/impl/snowflake/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 9 additions & 3 deletions dlt/destinations/impl/sqlalchemy/configuration.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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):
Expand Down
6 changes: 3 additions & 3 deletions tests/common/configuration/test_credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = """
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand Down
5 changes: 3 additions & 2 deletions tests/common/configuration/test_toml_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@

from tests.utils import preserve_environ
from tests.common.configuration.utils import (
ConnectionStringCompatCredentials,
SecretCredentials,
WithCredentialsConfiguration,
CoercionTestConfiguration,
Expand Down Expand Up @@ -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"))
Expand Down
7 changes: 7 additions & 0 deletions tests/common/configuration/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit a938752

Please sign in to comment.