Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/1858 make all connection string credentials optional #1867

Merged
merged 2 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading