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

Migrate to dbt-common + dbt-adapters #342

Merged
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
1e25859
Migrate to dbt-common + dbt-adapters
jtcohen6 Feb 19, 2024
86eface
Try different install reqs
jtcohen6 Feb 19, 2024
2d36968
Fix unit tests
jtcohen6 Feb 19, 2024
3ad787e
Bonus: functional tests for dbt unit testing
jtcohen6 Feb 19, 2024
d4d5939
bump dbt-common and dbt-adapters to 1.0.0b1
MichelleArk Feb 29, 2024
10034b3
Merge branch 'master' into jerco/migrate-dbt-common-adapters-interfaces
MichelleArk Feb 29, 2024
5712711
implement DuckDbRelation.create_from, fix TestExternalSources::test_e…
MichelleArk Feb 29, 2024
af5f989
Merge branch 'master' into jerco/migrate-dbt-common-adapters-interfaces
MichelleArk Mar 15, 2024
9c8113a
use RelationConfig attributes in create_from_source
MichelleArk Mar 15, 2024
80b709d
Merge branch 'master' into jerco/migrate-dbt-common-adapters-interfaces
MichelleArk Mar 19, 2024
b8fdca0
formatting + initial mypy fixes
MichelleArk Mar 19, 2024
6527752
Merge branch 'master' into jerco/migrate-dbt-common-adapters-interfaces
jtcohen6 Apr 9, 2024
9a0ba02
Revert dev-requirements
jtcohen6 Apr 10, 2024
6dcc39c
Readd dbt-tests-adapter
jtcohen6 Apr 10, 2024
5f594c4
Readd dbt-core to setup.py for install back-compat
jtcohen6 Apr 16, 2024
8511b94
Merge branch 'master' into jerco/migrate-dbt-common-adapters-interfaces
jtcohen6 Apr 16, 2024
0750e0c
Merge remote-tracking branch 'origin/master' into jerco/migrate-dbt-c…
jtcohen6 Apr 16, 2024
a85cef9
Skip BV for TestUnitTestingTypesDuckDB
jtcohen6 Apr 19, 2024
298f27c
Merge branch 'master' into jerco/migrate-dbt-common-adapters-interfaces
jwills May 3, 2024
08d5530
Merge branch 'master' into jerco/migrate-dbt-common-adapters-interfaces
jwills May 8, 2024
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
21 changes: 12 additions & 9 deletions dbt/adapters/duckdb/connections.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
import atexit
import threading
from contextlib import contextmanager
from multiprocessing.context import SpawnContext
from typing import Optional
from typing import Tuple
from typing import TYPE_CHECKING

import dbt.exceptions
from . import environments
from dbt.adapters.contracts.connection import AdapterRequiredConfig
from dbt.adapters.contracts.connection import AdapterResponse
from dbt.adapters.contracts.connection import Connection
from dbt.adapters.contracts.connection import ConnectionState
from dbt.adapters.events.logging import AdapterLogger
from dbt.adapters.sql import SQLConnectionManager
from dbt.contracts.connection import AdapterRequiredConfig
from dbt.contracts.connection import AdapterResponse
from dbt.contracts.connection import Connection
from dbt.contracts.connection import ConnectionState
from dbt.logger import GLOBAL_LOGGER as logger

logger = AdapterLogger("DuckDB")

if TYPE_CHECKING:
import agate
Expand All @@ -23,9 +26,9 @@ class DuckDBConnectionManager(SQLConnectionManager):
_LOCK = threading.RLock()
_ENV = None

def __init__(self, profile: AdapterRequiredConfig):
super().__init__(profile)
self.disable_transactions = profile.credentials.disable_transactions # type: ignore
def __init__(self, config: AdapterRequiredConfig, mp_context: SpawnContext) -> None:
super().__init__(config, mp_context)
self.disable_transactions = config.credentials.disable_transactions # type: ignore

@classmethod
def env(cls) -> environments.Environment:
Expand All @@ -52,7 +55,7 @@ def open(cls, connection: Connection) -> Connection:
logger.debug("Got an error when attempting to connect to DuckDB: '{}'".format(e))
connection.handle = None
connection.state = ConnectionState.FAIL
raise dbt.exceptions.FailedToConnectError(str(e))
raise dbt.adapters.exceptions.FailedToConnectError(str(e))

return connection

Expand Down
11 changes: 6 additions & 5 deletions dbt/adapters/duckdb/credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,10 @@
from typing import Tuple
from urllib.parse import urlparse

import dbt.exceptions
from dbt.adapters.base import Credentials
from dbt.dataclass_schema import dbtClassMixin
from dbt_common.dataclass_schema import dbtClassMixin
from dbt_common.exceptions import DbtRuntimeError

from dbt.adapters.contracts.connection import Credentials


@dataclass
Expand Down Expand Up @@ -187,12 +188,12 @@ def __pre_deserialize__(cls, data: Dict[Any, Any]) -> Dict[Any, Any]:
data["database"] = path_db
elif path_db and data["database"] != path_db:
if not data.get("remote"):
raise dbt.exceptions.DbtRuntimeError(
raise DbtRuntimeError(
"Inconsistency detected between 'path' and 'database' fields in profile; "
f"the 'database' property must be set to '{path_db}' to match the 'path'"
)
elif not path_db:
raise dbt.exceptions.DbtRuntimeError(
raise DbtRuntimeError(
"Unable to determine target database name from 'path' field in profile"
)
return data
Expand Down
4 changes: 2 additions & 2 deletions dbt/adapters/duckdb/environments/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
from typing import Optional

import duckdb
from dbt_common.exceptions import DbtRuntimeError

from ..credentials import DuckDBCredentials
from ..plugins import BasePlugin
from ..utils import SourceConfig
from ..utils import TargetConfig
from dbt.contracts.connection import AdapterResponse
from dbt.exceptions import DbtRuntimeError
from dbt.adapters.contracts.connection import AdapterResponse


def _ensure_event_loop():
Expand Down
2 changes: 1 addition & 1 deletion dbt/adapters/duckdb/environments/buenavista.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from . import Environment
from .. import credentials
from .. import utils
from dbt.contracts.connection import AdapterResponse
from dbt.adapters.contracts.connection import AdapterResponse


class BVEnvironment(Environment):
Expand Down
5 changes: 3 additions & 2 deletions dbt/adapters/duckdb/environments/local.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import threading

from dbt_common.exceptions import DbtRuntimeError

from . import Environment
from .. import credentials
from .. import utils
from dbt.contracts.connection import AdapterResponse
from dbt.exceptions import DbtRuntimeError
from dbt.adapters.contracts.connection import AdapterResponse


class DuckDBCursorWrapper:
Expand Down
19 changes: 10 additions & 9 deletions dbt/adapters/duckdb/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,25 @@
from typing import Sequence
from typing import TYPE_CHECKING

from dbt_common.contracts.constraints import ColumnLevelConstraint
from dbt_common.contracts.constraints import ConstraintType
from dbt_common.exceptions import DbtInternalError
from dbt_common.exceptions import DbtRuntimeError

from dbt.adapters.base import BaseRelation
from dbt.adapters.base.column import Column as BaseColumn
from dbt.adapters.base.impl import ConstraintSupport
from dbt.adapters.base.meta import available
from dbt.adapters.contracts.connection import AdapterResponse
from dbt.adapters.contracts.relation import Path
from dbt.adapters.contracts.relation import RelationType
from dbt.adapters.duckdb.column import DuckDBColumn
from dbt.adapters.duckdb.connections import DuckDBConnectionManager
from dbt.adapters.duckdb.relation import DuckDBRelation
from dbt.adapters.duckdb.utils import TargetConfig
from dbt.adapters.duckdb.utils import TargetLocation
from dbt.adapters.sql import SQLAdapter
from dbt.context.providers import RuntimeConfigObject
from dbt.contracts.connection import AdapterResponse
from dbt.contracts.graph.nodes import ColumnLevelConstraint
from dbt.contracts.graph.nodes import ConstraintType
from dbt.contracts.relation import Path
from dbt.contracts.relation import RelationType
from dbt.exceptions import DbtInternalError
from dbt.exceptions import DbtRuntimeError


TEMP_SCHEMA_NAME = "temp_schema_name"
DEFAULT_TEMP_SCHEMA_NAME = "dbt_temp"
Expand Down Expand Up @@ -103,7 +104,7 @@ def store_relation(
column_list: Sequence[BaseColumn],
path: str,
format: str,
config: RuntimeConfigObject,
config: Any,
) -> None:
target_config = TargetConfig(
relation=relation,
Expand Down
2 changes: 1 addition & 1 deletion dbt/adapters/duckdb/plugins/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
from typing import Dict
from typing import Optional

from dbt_common.dataclass_schema import dbtClassMixin
from duckdb import DuckDBPyConnection

from ..credentials import DuckDBCredentials
from ..utils import SourceConfig
from ..utils import TargetConfig
from dbt.dataclass_schema import dbtClassMixin


class PluginConfig(dbtClassMixin):
Expand Down
22 changes: 18 additions & 4 deletions dbt/adapters/duckdb/relation.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,30 @@
from .connections import DuckDBConnectionManager
from .utils import SourceConfig
from dbt.adapters.base.relation import BaseRelation
from dbt.adapters.base.relation import Self
from dbt.contracts.graph.nodes import SourceDefinition
from dbt.adapters.contracts.relation import HasQuoting
from dbt.adapters.contracts.relation import RelationConfig


@dataclass(frozen=True, eq=False, repr=False)
class DuckDBRelation(BaseRelation):
external: Optional[str] = None

@classmethod
def create_from_source(cls: Type[Self], source: SourceDefinition, **kwargs: Any) -> Self:
def create_from(
cls: Type["DuckDBRelation"],
quoting: HasQuoting,
relation_config: RelationConfig,
**kwargs: Any,
) -> "DuckDBRelation":
if relation_config.resource_type == "source":
return cls.create_from_source(quoting, relation_config, **kwargs)
else:
return super().create_from(quoting, relation_config, **kwargs)

@classmethod
def create_from_source(
cls: Type["DuckDBRelation"], quoting: HasQuoting, source: RelationConfig, **kwargs: Any
) -> "DuckDBRelation":
"""
This method creates a new DuckDBRelation instance from a source definition.
It first checks if a 'plugin' is defined in the meta argument for the source or its parent configuration.
Expand Down Expand Up @@ -59,7 +73,7 @@ def create_from_source(cls: Type[Self], source: SourceDefinition, **kwargs: Any)
ext_location = f"'{ext_location}'"
kwargs["external"] = ext_location

return super().create_from_source(source, **kwargs) # type: ignore
return super().create_from(quoting, source, **kwargs) # type: ignore

def render(self) -> str:
if self.external:
Expand Down
15 changes: 8 additions & 7 deletions dbt/adapters/duckdb/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,9 @@

from dbt.adapters.base.column import Column
from dbt.adapters.base.relation import BaseRelation
from dbt.context.providers import RuntimeConfigObject
from dbt.contracts.graph.nodes import SourceDefinition
from dbt.adapters.contracts.relation import RelationConfig
# TODO
# from dbt.context.providers import RuntimeConfigObject


@dataclass
Expand Down Expand Up @@ -47,11 +48,11 @@ def as_dict(self) -> Dict[str, Any]:
return base

@classmethod
def create_from_source(cls, source: SourceDefinition) -> "SourceConfig":
meta = source.source_meta.copy()
meta.update(source.meta)
def create_from_source(cls, source: RelationConfig) -> "SourceConfig":
meta = source.meta.copy()
# Use the config properties as well if they are present
meta.update(source.config._extra)
config_properties = source.config.extra if source.config else {}
meta.update(config_properties)
return SourceConfig(
name=source.name,
identifier=source.identifier,
Expand All @@ -75,7 +76,7 @@ def as_dict(self) -> Dict[str, Any]:
class TargetConfig:
relation: BaseRelation
column_list: Sequence[Column]
config: RuntimeConfigObject
config: Any # TODO
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably the safest since our usage of this config object is pretty much limited to get methods

location: Optional[TargetLocation] = None

def as_dict(self) -> Dict[str, Any]:
Expand Down
6 changes: 3 additions & 3 deletions dev-requirements.txt
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# install latest changes in dbt-core
# install latest changes in dbt-core + dbt-tests-adapter
# git+https://github.com/dbt-labs/dbt-core.git#egg=dbt-core&subdirectory=core
# git+https://github.com/dbt-labs/dbt-core.git#egg=dbt-tests-adapter&subdirectory=tests/adapter
# git+https://github.com/dbt-labs/dbt-adapters.git#subdirectory=dbt-tests-adapter

dbt-tests-adapter==1.7.11
dbt-tests-adapter>=1.8.0b1

boto3
mypy-boto3-glue
Expand Down
5 changes: 4 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,11 @@ def _dbt_duckdb_version():
packages=find_namespace_packages(include=["dbt", "dbt.*"]),
include_package_data=True,
install_requires=[
"dbt-core~=1.7.0",
"dbt-common>=1,<2",
"dbt-adapters>=1,<2",
"duckdb>=0.7.0",
# add dbt-core to ensure backwards compatibility of installation, this is not a functional dependency
"dbt-core>=1.8.0b1",
Comment on lines +43 to +44
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though dbt-core is no longer a functional dependency of adapters, we're going to keep including dbt-core in setup.py to prevent surprising/breaking changes to the behavior of pip install dbt-duckdb on the day of the v1.8 final release.

dbt-duck shouldn't import anything from dbt-core directly, only from the interfaces defined in dbt-core + dbt-adapters.

],
extras_require={"glue": ["boto3", "mypy-boto3-glue"], "md": ["duckdb>=0.7.0,<=0.9.2"]},
classifiers=[
Expand Down
35 changes: 35 additions & 0 deletions tests/functional/adapter/test_unit_testing.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import pytest

from dbt.tests.adapter.unit_testing.test_types import BaseUnitTestingTypes
from dbt.tests.adapter.unit_testing.test_case_insensitivity import BaseUnitTestCaseInsensivity
from dbt.tests.adapter.unit_testing.test_invalid_input import BaseUnitTestInvalidInput


class TestUnitTestingTypesDuckDB(BaseUnitTestingTypes):
Copy link
Contributor Author

@jtcohen6 jtcohen6 Apr 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jwills I pulled these in (selfishly), since it seemed like unit testing might "just work" on dbt-duckdb with the v1.8 interface updates.

It's all well & good on the Real Duck, but it looks like the test is failing on BuenaVista for the last data type in the list (ARRAY[1,2,3]):

  File "/home/runner/work/dbt-duckdb/dbt-duckdb/.tox/buenavista/lib/python3.9/site-packages/buenavista/postgres.py", line 104, in <lambda>
    BVType.INTEGERARRAY: (1007, lambda v: "{" + ",".join(v) + "}"),
TypeError: sequence item 0: expected str instance, int found
sequence item 0: expected str instance, int found
---------------------- CSV report: buenavista_results.csv ----------------------
=========================== short test summary info ============================
FAILED tests/functional/adapter/test_unit_testing.py::TestUnitTestingTypesDuckDB::test_unit_test_data_type - AssertionError: unit test failed when testing model with ARRAY[1,2,3]

How would you feel about me adding @pytest.mark.skip_profile("buenavista") to this test? I know that adds a bit of tech debt / mystery for you later on

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that’s just because I don’t properly support array types in the BV Postgres serialization, so it’s perfectly understandable and okay to mark it as skipped

@pytest.fixture
def data_types(self):
# sql_value, yaml_value
return [
["1", "1"],
["2.0", "2.0"],
["'12345'", "12345"],
["'string'", "string"],
["true", "true"],
["DATE '2020-01-02'", "2020-01-02"],
["TIMESTAMP '2013-11-03 00:00:00-0'", "2013-11-03 00:00:00-0"],
["'2013-11-03 00:00:00-0'::TIMESTAMPTZ", "2013-11-03 00:00:00-0"],
[
"{'Alberta':'Edmonton','Manitoba':'Winnipeg'}",
"{'Alberta':'Edmonton','Manitoba':'Winnipeg'}",
],
["ARRAY['a','b','c']", "['a','b','c']"],
["ARRAY[1,2,3]", "[1, 2, 3]"],
]


class TestUnitTestCaseInsensitivityDuckDB(BaseUnitTestCaseInsensivity):
pass


class TestUnitTestInvalidInputDuckDB(BaseUnitTestInvalidInput):
pass
3 changes: 2 additions & 1 deletion tests/unit/test_duckdb_adapter.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,9 @@ def setUp(self):

@property
def adapter(self):
self.mock_mp_context = mock.MagicMock()
if self._adapter is None:
self._adapter = DuckDBAdapter(self.config)
self._adapter = DuckDBAdapter(self.config, self.mock_mp_context)
return self._adapter

@mock.patch("dbt.adapters.duckdb.environments.duckdb")
Expand Down
4 changes: 3 additions & 1 deletion tests/unit/test_external_utils.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import unittest
from argparse import Namespace
from unittest import mock

from dbt.flags import set_from_args
from dbt.adapters.duckdb import DuckDBAdapter
Expand Down Expand Up @@ -32,8 +33,9 @@ def setUp(self):

@property
def adapter(self):
self.mock_mp_context = mock.MagicMock()
if self._adapter is None:
self._adapter = DuckDBAdapter(self.config)
self._adapter = DuckDBAdapter(self.config, self.mock_mp_context)
return self._adapter

def test_external_write_options(self):
Expand Down
Loading