From ee629f7d90bec288962528336cdd39af529f0f67 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Mon, 12 Aug 2024 17:10:49 -0700 Subject: [PATCH 01/17] point to the dev branch on dbt_common --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index e794781c..8466eed5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,7 +54,7 @@ include = ["dbt/adapters", "dbt/include", "dbt/__init__.py"] [tool.hatch.envs.default] dependencies = [ - "dbt_common @ git+https://github.com/dbt-labs/dbt-common.git", + "dbt_common @ git+https://github.com/dbt-labs/dbt-common.git@behavior-flags", 'pre-commit==3.7.0;python_version>="3.9"', 'pre-commit==3.5.0;python_version=="3.8"', "pytest", From 97545d841475962e90601dc48fbd99bd21a76c00 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Mon, 12 Aug 2024 17:16:27 -0700 Subject: [PATCH 02/17] add behavior flags to the base adapter --- dbt/adapters/base/impl.py | 11 +++++++++++ dbt/adapters/behavior_flags.py | 6 ++++++ 2 files changed, 17 insertions(+) create mode 100644 dbt/adapters/behavior_flags.py diff --git a/dbt/adapters/base/impl.py b/dbt/adapters/base/impl.py index e0627c47..a3dcfb7b 100644 --- a/dbt/adapters/base/impl.py +++ b/dbt/adapters/base/impl.py @@ -24,6 +24,7 @@ ) import pytz +from dbt_common.behavior_flags import Behavior, RawBehaviorFlag, register from dbt_common.clients.jinja import CallableMacroGenerator from dbt_common.contracts.constraints import ( ColumnLevelConstraint, @@ -61,6 +62,7 @@ InformationSchema, SchemaSearchMap, ) +from dbt.adapters.behavior_flags import flags as base_flags from dbt.adapters.cache import RelationsCache, _make_ref_key_dict from dbt.adapters.capability import Capability, CapabilityDict from dbt.adapters.contracts.connection import Credentials @@ -271,6 +273,7 @@ def __init__(self, config, mp_context: SpawnContext) -> None: self.connections = self.ConnectionManager(config, mp_context) self._macro_resolver: Optional[MacroResolverProtocol] = None self._macro_context_generator: Optional[MacroContextGeneratorCallable] = None + self.behavior = self.register_behavior_flags() ### # Methods to set / access a macro resolver @@ -291,6 +294,14 @@ def set_macro_context_generator( ) -> None: self._macro_context_generator = macro_context_generator + def register_behavior_flags(self) -> Behavior: + behavior_flags = base_flags.copy() + behavior_flags.extend(self._include_behavior_flags()) + return register(behavior_flags, self.config.flags) + + def _include_behavior_flags(self) -> List[RawBehaviorFlag]: + return [] + ### # Methods that pass through to the connection manager ### diff --git a/dbt/adapters/behavior_flags.py b/dbt/adapters/behavior_flags.py new file mode 100644 index 00000000..9e39d208 --- /dev/null +++ b/dbt/adapters/behavior_flags.py @@ -0,0 +1,6 @@ +from typing import List + +from dbt_common.behavior_flags import RawBehaviorFlag + + +flags: List[RawBehaviorFlag] = [] From a858aa35a6863af5399f1b7c835c4e067d6b985e Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Sun, 18 Aug 2024 00:44:12 -0400 Subject: [PATCH 03/17] simplify initial imlementation until we get a real behavior flag to implement --- dbt/adapters/base/impl.py | 11 +++++++---- dbt/adapters/behavior_flags.py | 6 ------ 2 files changed, 7 insertions(+), 10 deletions(-) delete mode 100644 dbt/adapters/behavior_flags.py diff --git a/dbt/adapters/base/impl.py b/dbt/adapters/base/impl.py index a3dcfb7b..058a9db9 100644 --- a/dbt/adapters/base/impl.py +++ b/dbt/adapters/base/impl.py @@ -62,7 +62,6 @@ InformationSchema, SchemaSearchMap, ) -from dbt.adapters.behavior_flags import flags as base_flags from dbt.adapters.cache import RelationsCache, _make_ref_key_dict from dbt.adapters.capability import Capability, CapabilityDict from dbt.adapters.contracts.connection import Credentials @@ -295,11 +294,15 @@ def set_macro_context_generator( self._macro_context_generator = macro_context_generator def register_behavior_flags(self) -> Behavior: - behavior_flags = base_flags.copy() - behavior_flags.extend(self._include_behavior_flags()) - return register(behavior_flags, self.config.flags) + """ + Collect all raw behavior flags and produce a behavior namespace + """ + return register(self._include_behavior_flags(), self.config.flags) def _include_behavior_flags(self) -> List[RawBehaviorFlag]: + """ + An optional abstract method that concrete adapters can use to inject platform-specific behavior flags + """ return [] ### diff --git a/dbt/adapters/behavior_flags.py b/dbt/adapters/behavior_flags.py deleted file mode 100644 index 9e39d208..00000000 --- a/dbt/adapters/behavior_flags.py +++ /dev/null @@ -1,6 +0,0 @@ -from typing import List - -from dbt_common.behavior_flags import RawBehaviorFlag - - -flags: List[RawBehaviorFlag] = [] From d113494aedd81ea25d05fd381887dba6165f3d6d Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Sun, 18 Aug 2024 00:52:01 -0400 Subject: [PATCH 04/17] changelog --- .changes/unreleased/Features-20240818-005131.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/Features-20240818-005131.yaml diff --git a/.changes/unreleased/Features-20240818-005131.yaml b/.changes/unreleased/Features-20240818-005131.yaml new file mode 100644 index 00000000..17260206 --- /dev/null +++ b/.changes/unreleased/Features-20240818-005131.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Add Behavior Flag framework +time: 2024-08-18T00:51:31.753656-04:00 +custom: + Author: mikealfare + Issue: "281" From 2504b55ab216641b73f1c5ceb0117aea357b98ee Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 23 Aug 2024 20:34:14 -0400 Subject: [PATCH 05/17] simplify adding behavior flags to an adapter and allow for easier testing --- dbt/adapters/base/impl.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/dbt/adapters/base/impl.py b/dbt/adapters/base/impl.py index 058a9db9..eedf5c70 100644 --- a/dbt/adapters/base/impl.py +++ b/dbt/adapters/base/impl.py @@ -272,7 +272,7 @@ def __init__(self, config, mp_context: SpawnContext) -> None: self.connections = self.ConnectionManager(config, mp_context) self._macro_resolver: Optional[MacroResolverProtocol] = None self._macro_context_generator: Optional[MacroContextGeneratorCallable] = None - self.behavior = self.register_behavior_flags() + self.behavior = [] # this will be updated to include global behavior flags once they exist ### # Methods to set / access a macro resolver @@ -293,15 +293,19 @@ def set_macro_context_generator( ) -> None: self._macro_context_generator = macro_context_generator - def register_behavior_flags(self) -> Behavior: - """ - Collect all raw behavior flags and produce a behavior namespace - """ - return register(self._include_behavior_flags(), self.config.flags) + @property + def behavior(self) -> Behavior: + return self._behavior - def _include_behavior_flags(self) -> List[RawBehaviorFlag]: + @behavior.setter + def behavior(self, raw_behavior_flags: List[RawBehaviorFlag]) -> None: + raw_behavior_flags.extend(self._behavior_extra) + self._behavior = register(raw_behavior_flags, self.config.flags) + + @property + def _behavior_extra(self) -> List[RawBehaviorFlag]: """ - An optional abstract method that concrete adapters can use to inject platform-specific behavior flags + This method should be overwritten by adapter maintainers to provide platform-specific flags """ return [] From ada035115597cc2b5ab2e8ca13e5068660376a1b Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 23 Aug 2024 20:34:47 -0400 Subject: [PATCH 06/17] implement an adapter stub for unit testing --- tests/unit/conftest.py | 199 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 199 insertions(+) create mode 100644 tests/unit/conftest.py diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py new file mode 100644 index 00000000..4dc37639 --- /dev/null +++ b/tests/unit/conftest.py @@ -0,0 +1,199 @@ +from contextlib import contextmanager +from multiprocessing import get_context +from types import SimpleNamespace +from typing import Any, ContextManager, Dict, List, Optional, Tuple + +import agate +from dbt_common.behavior_flags import RawBehaviorFlag +import pytest + +from dbt.adapters.base.column import Column +from dbt.adapters.base.connections import BaseConnectionManager +from dbt.adapters.base.impl import BaseAdapter +from dbt.adapters.base.relation import BaseRelation +from dbt.adapters.contracts.connection import ( + AdapterRequiredConfig, + AdapterResponse, + Connection, + ConnectionState, + Credentials, + QueryComment, +) + + +@pytest.fixture +def adapter(config, existing_relations, behavior_flags) -> BaseAdapter: + + adapter = BaseAdapterStub(config, get_context("spawn")) + + adapter.behavior = behavior_flags + + for relation in existing_relations: + adapter.cache_added(BaseRelation.create(relation)) + + return adapter + + +@pytest.fixture +def config(config_extra) -> AdapterRequiredConfig: + raw_config = { + "credentials": CredentialsStub("test_database", "test_schema"), + "profile_name": "test_profile", + "target_name": "test_target", + "threads": 4, + "project_name": "test_project", + "query_comment": QueryComment(), + "cli_vars": {}, + "target_path": "path/to/nowhere", + "log_cache_events": False, + } + raw_config.update(config_extra) + return SimpleNamespace(**raw_config) + + +@pytest.fixture +def config_extra() -> Dict[str, Any]: + return {} + + +@pytest.fixture +def existing_relations() -> List[Dict[str, str]]: + return [] + + +@pytest.fixture +def behavior_flags() -> List[RawBehaviorFlag]: + return [] + + +class CredentialsStub(Credentials): + def type(self) -> str: + return "test" + + def _connection_keys(self): + return {"database": self.database, "schema": self.schema} + + +class ConnectionManagerStub(BaseConnectionManager): + + @contextmanager + def exception_handler(self, sql: str) -> ContextManager: # type: ignore + try: + yield + finally: + pass + + def cancel_open(self) -> Optional[List[str]]: + # there's no database, so there are no connections + pass + + def open(cls, connection: Connection) -> Connection: + # there's no database, so just change the state + connection.state = ConnectionState.OPEN + return connection + + def begin(self) -> None: + # there's no database, so there are no transactions + pass + + def commit(self) -> None: + # there's no database, so there are no transactions + pass + + def execute( + self, + sql: str, + auto_begin: bool = False, + fetch: bool = False, + limit: Optional[int] = None, + ) -> Tuple[AdapterResponse, agate.Table]: + # there's no database, so just return the sql + return AdapterResponse(code=sql), None + + +class BaseAdapterStub(BaseAdapter): + + ConnectionManager = ConnectionManagerStub + + ### + # Abstract methods for database-specific values, attributes, and types + ### + @classmethod + def date_function(cls) -> str: + return "date_function" + + @classmethod + def is_cancelable(cls) -> bool: + return False + + def list_schemas(self, database: str) -> List[str]: + return list(self.cache.schemas) + + ### + # Abstract methods about relations + ### + def drop_relation(self, relation: BaseRelation) -> None: + self.cache_dropped(relation) + + def truncate_relation(self, relation: BaseRelation) -> None: + self.cache_dropped(relation) + + def rename_relation(self, from_relation: BaseRelation, to_relation: BaseRelation) -> None: + self.cache_renamed(from_relation, to_relation) + + def get_columns_in_relation(self, relation: BaseRelation) -> List[Column]: + # there's no database, so these need to be added as kwargs in the existing_relations fixture + return relation.columns + + def expand_column_types(self, goal: BaseRelation, current: BaseRelation) -> None: + # there's no database, so these need to be added as kwargs in the existing_relations fixture + current.columns = goal.columns + + def list_relations_without_caching(self, schema_relation: BaseRelation) -> List[BaseRelation]: + # there's no database, so use the cache as the database + return self.cache.get_relations(schema_relation.database, schema_relation.schema) + + ### + # ODBC FUNCTIONS -- these should not need to change for every adapter, + # although some adapters may override them + ### + def create_schema(self, relation: BaseRelation): + # there's no database, this happens implicitly by adding a relation to the cache + pass + + def drop_schema(self, relation: BaseRelation): + for each_relation in self.cache.get_relations(relation.database, relation.schema): + self.cache_dropped(each_relation) + + @classmethod + def quote(cls, identifier: str) -> str: + quote_char = "" + return f"{quote_char}{identifier}{quote_char}" + + ### + # Conversions: These must be implemented by concrete implementations, for + # converting agate types into their sql equivalents. + ### + @classmethod + def convert_text_type(cls, agate_table: agate.Table, col_idx: int) -> str: + return "str" + + @classmethod + def convert_number_type(cls, agate_table: agate.Table, col_idx: int) -> str: + return "float" + + @classmethod + def convert_boolean_type(cls, agate_table: agate.Table, col_idx: int) -> str: + return "bool" + + @classmethod + def convert_datetime_type(cls, agate_table: agate.Table, col_idx: int) -> str: + return "datetime" + + @classmethod + def convert_date_type(cls, *args, **kwargs): + return "date" + + @classmethod + def convert_time_type(cls, *args, **kwargs): + return "time" From 35695e0b0315f04cea418ba1b7bcfeded8f5745f Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 23 Aug 2024 20:35:15 -0400 Subject: [PATCH 07/17] add unit tests for behavior flags --- tests/unit/test_behavior_flags.py | 41 +++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) create mode 100644 tests/unit/test_behavior_flags.py diff --git a/tests/unit/test_behavior_flags.py b/tests/unit/test_behavior_flags.py new file mode 100644 index 00000000..6a11dce6 --- /dev/null +++ b/tests/unit/test_behavior_flags.py @@ -0,0 +1,41 @@ +from typing import Any, Dict, List + +from dbt_common.behavior_flags import RawBehaviorFlag +import pytest + + +@pytest.fixture +def config_extra() -> Dict[str, Any]: + config = { + "flags": { + "unregistered_flag": True, + "default_false_user_false_flag": False, + "default_false_user_true_flag": True, + "default_true_user_false_flag": False, + "default_true_user_true_flag": True, + } + } + return config + + +@pytest.fixture +def behavior_flags() -> List[RawBehaviorFlag]: + return [ + {"name": "default_false_user_false_flag", "default": False}, + {"name": "default_false_user_true_flag", "default": False}, + {"name": "default_false_user_skip_flag", "default": False}, + {"name": "default_true_user_false_flag", "default": True}, + {"name": "default_true_user_true_flag", "default": True}, + {"name": "default_true_user_skip_flag", "default": True}, + ] + + +def test_register_behavior_flags(mocker, adapter): + with pytest.raises(AttributeError): + assert adapter.behavior.unregistered_flag + assert not adapter.behavior.default_false_user_false_flag + assert adapter.behavior.default_false_user_true_flag + assert not adapter.behavior.default_false_user_skip_flag + assert not adapter.behavior.default_true_user_false_flag + assert adapter.behavior.default_true_user_true_flag + assert adapter.behavior.default_true_user_skip_flag From c292bcd8e09e77286de9979ef3befcd62415c654 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 23 Aug 2024 21:03:28 -0400 Subject: [PATCH 08/17] remove unused fixture --- tests/unit/test_behavior_flags.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/test_behavior_flags.py b/tests/unit/test_behavior_flags.py index 6a11dce6..fed1f86f 100644 --- a/tests/unit/test_behavior_flags.py +++ b/tests/unit/test_behavior_flags.py @@ -30,7 +30,7 @@ def behavior_flags() -> List[RawBehaviorFlag]: ] -def test_register_behavior_flags(mocker, adapter): +def test_register_behavior_flags(adapter): with pytest.raises(AttributeError): assert adapter.behavior.unregistered_flag assert not adapter.behavior.default_false_user_false_flag From ef20cdb66cc36bf84967aeaeec5286a43e97b8ad Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Sat, 24 Aug 2024 17:47:15 -0400 Subject: [PATCH 09/17] update call to create behavior --- dbt/adapters/base/impl.py | 4 ++-- tests/unit/test_behavior_flags.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/dbt/adapters/base/impl.py b/dbt/adapters/base/impl.py index eedf5c70..9deb1846 100644 --- a/dbt/adapters/base/impl.py +++ b/dbt/adapters/base/impl.py @@ -24,7 +24,7 @@ ) import pytz -from dbt_common.behavior_flags import Behavior, RawBehaviorFlag, register +from dbt_common.behavior_flags import Behavior, RawBehaviorFlag from dbt_common.clients.jinja import CallableMacroGenerator from dbt_common.contracts.constraints import ( ColumnLevelConstraint, @@ -300,7 +300,7 @@ def behavior(self) -> Behavior: @behavior.setter def behavior(self, raw_behavior_flags: List[RawBehaviorFlag]) -> None: raw_behavior_flags.extend(self._behavior_extra) - self._behavior = register(raw_behavior_flags, self.config.flags) + self._behavior = Behavior(raw_behavior_flags, self.config.flags) @property def _behavior_extra(self) -> List[RawBehaviorFlag]: diff --git a/tests/unit/test_behavior_flags.py b/tests/unit/test_behavior_flags.py index fed1f86f..c3e7697c 100644 --- a/tests/unit/test_behavior_flags.py +++ b/tests/unit/test_behavior_flags.py @@ -1,6 +1,7 @@ from typing import Any, Dict, List from dbt_common.behavior_flags import RawBehaviorFlag +from dbt_common.exceptions import DbtInternalError import pytest @@ -31,7 +32,7 @@ def behavior_flags() -> List[RawBehaviorFlag]: def test_register_behavior_flags(adapter): - with pytest.raises(AttributeError): + with pytest.raises(DbtInternalError): assert adapter.behavior.unregistered_flag assert not adapter.behavior.default_false_user_false_flag assert adapter.behavior.default_false_user_true_flag From 2631fbf58a59d88699f36b6653a32697ed52864d Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 29 Aug 2024 18:24:08 -0400 Subject: [PATCH 10/17] update behavior flag name as a result of adopting the Rendered naming convention --- dbt/adapters/base/impl.py | 8 ++++---- tests/unit/conftest.py | 6 +++--- tests/unit/test_behavior_flags.py | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/dbt/adapters/base/impl.py b/dbt/adapters/base/impl.py index 9deb1846..581e7f16 100644 --- a/dbt/adapters/base/impl.py +++ b/dbt/adapters/base/impl.py @@ -24,7 +24,7 @@ ) import pytz -from dbt_common.behavior_flags import Behavior, RawBehaviorFlag +from dbt_common.behavior_flags import Behavior, BehaviorFlag from dbt_common.clients.jinja import CallableMacroGenerator from dbt_common.contracts.constraints import ( ColumnLevelConstraint, @@ -262,7 +262,7 @@ class BaseAdapter(metaclass=AdapterMeta): MAX_SCHEMA_METADATA_RELATIONS = 100 - # This static member variable can be overriden in concrete adapter + # This static member variable can be overridden in concrete adapter # implementations to indicate adapter support for optional capabilities. _capabilities = CapabilityDict({}) @@ -298,12 +298,12 @@ def behavior(self) -> Behavior: return self._behavior @behavior.setter - def behavior(self, raw_behavior_flags: List[RawBehaviorFlag]) -> None: + def behavior(self, raw_behavior_flags: List[BehaviorFlag]) -> None: raw_behavior_flags.extend(self._behavior_extra) self._behavior = Behavior(raw_behavior_flags, self.config.flags) @property - def _behavior_extra(self) -> List[RawBehaviorFlag]: + def _behavior_extra(self) -> List[BehaviorFlag]: """ This method should be overwritten by adapter maintainers to provide platform-specific flags """ diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 4dc37639..817ba2eb 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -4,7 +4,7 @@ from typing import Any, ContextManager, Dict, List, Optional, Tuple import agate -from dbt_common.behavior_flags import RawBehaviorFlag +from dbt_common.behavior_flags import BehaviorFlag import pytest from dbt.adapters.base.column import Column @@ -62,7 +62,7 @@ def existing_relations() -> List[Dict[str, str]]: @pytest.fixture -def behavior_flags() -> List[RawBehaviorFlag]: +def behavior_flags() -> List[BehaviorFlag]: return [] @@ -147,7 +147,7 @@ def get_columns_in_relation(self, relation: BaseRelation) -> List[Column]: def expand_column_types(self, goal: BaseRelation, current: BaseRelation) -> None: # there's no database, so these need to be added as kwargs in the existing_relations fixture - current.columns = goal.columns + object.__setattr__(current, "columns", goal.columns) def list_relations_without_caching(self, schema_relation: BaseRelation) -> List[BaseRelation]: # there's no database, so use the cache as the database diff --git a/tests/unit/test_behavior_flags.py b/tests/unit/test_behavior_flags.py index c3e7697c..3fc048f2 100644 --- a/tests/unit/test_behavior_flags.py +++ b/tests/unit/test_behavior_flags.py @@ -1,6 +1,6 @@ from typing import Any, Dict, List -from dbt_common.behavior_flags import RawBehaviorFlag +from dbt_common.behavior_flags import BehaviorFlag from dbt_common.exceptions import DbtInternalError import pytest @@ -20,7 +20,7 @@ def config_extra() -> Dict[str, Any]: @pytest.fixture -def behavior_flags() -> List[RawBehaviorFlag]: +def behavior_flags() -> List[BehaviorFlag]: return [ {"name": "default_false_user_false_flag", "default": False}, {"name": "default_false_user_true_flag", "default": False}, From 299ef1e77976dcc46965111fd7e471d5f8e3f7c5 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 29 Aug 2024 18:34:37 -0400 Subject: [PATCH 11/17] don't rely on config.flags being available --- dbt/adapters/base/impl.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/dbt/adapters/base/impl.py b/dbt/adapters/base/impl.py index 581e7f16..e21bcae4 100644 --- a/dbt/adapters/base/impl.py +++ b/dbt/adapters/base/impl.py @@ -298,9 +298,15 @@ def behavior(self) -> Behavior: return self._behavior @behavior.setter - def behavior(self, raw_behavior_flags: List[BehaviorFlag]) -> None: - raw_behavior_flags.extend(self._behavior_extra) - self._behavior = Behavior(raw_behavior_flags, self.config.flags) + def behavior(self, flags: List[BehaviorFlag]) -> None: + flags.extend(self._behavior_extra) + try: + # we don't always get project flags, for example during `dbt debug` + user_overrides = self.config.flags + except AttributeError: + # in that case, take the defaults + user_overrides = {} + self._behavior = Behavior(flags, user_overrides) @property def _behavior_extra(self) -> List[BehaviorFlag]: From 58d3baa5e8ef446d88bd810314b5c32073894349 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 29 Aug 2024 18:37:23 -0400 Subject: [PATCH 12/17] don't rely on config.flags being available --- dbt/adapters/base/impl.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/dbt/adapters/base/impl.py b/dbt/adapters/base/impl.py index e21bcae4..ba666261 100644 --- a/dbt/adapters/base/impl.py +++ b/dbt/adapters/base/impl.py @@ -302,11 +302,10 @@ def behavior(self, flags: List[BehaviorFlag]) -> None: flags.extend(self._behavior_extra) try: # we don't always get project flags, for example during `dbt debug` - user_overrides = self.config.flags + self._behavior = Behavior(flags, self.config.flags) except AttributeError: - # in that case, take the defaults - user_overrides = {} - self._behavior = Behavior(flags, user_overrides) + # in that case, don't load any behavior to avoid unexpected defaults + self._behavior = Behavior([], {}) @property def _behavior_extra(self) -> List[BehaviorFlag]: From 002031450007ca46d695f601be428914dcb22a3d Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Wed, 4 Sep 2024 19:28:04 -0400 Subject: [PATCH 13/17] minor updates for typing --- tests/unit/conftest.py | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 817ba2eb..7634f625 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -29,7 +29,7 @@ def adapter(config, existing_relations, behavior_flags) -> BaseAdapter: adapter.behavior = behavior_flags for relation in existing_relations: - adapter.cache_added(BaseRelation.create(relation)) + adapter.cache_added(BaseRelation.create(**relation)) return adapter @@ -67,6 +67,10 @@ def behavior_flags() -> List[BehaviorFlag]: class CredentialsStub(Credentials): + """ + A stub for a database credentials that does not connect to a database + """ + def type(self) -> str: return "test" @@ -75,18 +79,32 @@ def _connection_keys(self): class ConnectionManagerStub(BaseConnectionManager): + """ + A stub for a connection manager that does not connect to a database + """ + + raised_exceptions: List[Exception] @contextmanager def exception_handler(self, sql: str) -> ContextManager: # type: ignore + # catch all exceptions and put them on this class for inspection in tests try: yield + except Exception as exc: + self.raised_exceptions.append(exc) finally: pass def cancel_open(self) -> Optional[List[str]]: - # there's no database, so there are no connections - pass + names = [] + for connection in self.thread_connections.values(): + if connection.state == ConnectionState.OPEN: + connection.state = ConnectionState.CLOSED + if name := connection.name: + names.append(name) + return names + @classmethod def open(cls, connection: Connection) -> Connection: # there's no database, so just change the state connection.state = ConnectionState.OPEN @@ -108,10 +126,13 @@ def execute( limit: Optional[int] = None, ) -> Tuple[AdapterResponse, agate.Table]: # there's no database, so just return the sql - return AdapterResponse(code=sql), None + return AdapterResponse(_message="", code=sql), agate.Table([]) class BaseAdapterStub(BaseAdapter): + """ + A stub for an adapter that uses the cache as the database + """ ConnectionManager = ConnectionManagerStub From 405e6f1e81d50c6b53724de1aedb2f7cae6d3369 Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 5 Sep 2024 19:05:18 -0400 Subject: [PATCH 14/17] rename maintainer behavior flags method to a more specific name --- dbt/adapters/base/impl.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dbt/adapters/base/impl.py b/dbt/adapters/base/impl.py index ba666261..541c9846 100644 --- a/dbt/adapters/base/impl.py +++ b/dbt/adapters/base/impl.py @@ -299,7 +299,7 @@ def behavior(self) -> Behavior: @behavior.setter def behavior(self, flags: List[BehaviorFlag]) -> None: - flags.extend(self._behavior_extra) + flags.extend(self._behavior_flags) try: # we don't always get project flags, for example during `dbt debug` self._behavior = Behavior(flags, self.config.flags) @@ -308,7 +308,7 @@ def behavior(self, flags: List[BehaviorFlag]) -> None: self._behavior = Behavior([], {}) @property - def _behavior_extra(self) -> List[BehaviorFlag]: + def _behavior_flags(self) -> List[BehaviorFlag]: """ This method should be overwritten by adapter maintainers to provide platform-specific flags """ From a89d06655deb24a9ba3255348e7843add460e21f Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 5 Sep 2024 19:09:08 -0400 Subject: [PATCH 15/17] clarify why we are testing an exception that should be tested in dbt-common, test the broadest exception since the specific exception does not matter here --- tests/unit/test_behavior_flags.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_behavior_flags.py b/tests/unit/test_behavior_flags.py index 3fc048f2..b43e05b3 100644 --- a/tests/unit/test_behavior_flags.py +++ b/tests/unit/test_behavior_flags.py @@ -1,7 +1,7 @@ from typing import Any, Dict, List from dbt_common.behavior_flags import BehaviorFlag -from dbt_common.exceptions import DbtInternalError +from dbt_common.exceptions import DbtBaseException import pytest @@ -32,8 +32,11 @@ def behavior_flags() -> List[BehaviorFlag]: def test_register_behavior_flags(adapter): - with pytest.raises(DbtInternalError): + # make sure that users cannot add arbitrary flags to this collection + with pytest.raises(DbtBaseException): assert adapter.behavior.unregistered_flag + + # check the values of the valid behavior flags assert not adapter.behavior.default_false_user_false_flag assert adapter.behavior.default_false_user_true_flag assert not adapter.behavior.default_false_user_skip_flag From d63b2d2cbc2cbb1cbab9220e697446506afee75d Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Thu, 5 Sep 2024 20:19:05 -0400 Subject: [PATCH 16/17] break up fixtures into separate files --- .pre-commit-config.yaml | 2 +- tests/unit/conftest.py | 221 +--------------------- tests/unit/fixtures/__init__.py | 1 + tests/unit/fixtures/adapter.py | 146 ++++++++++++++ tests/unit/fixtures/connection_manager.py | 58 ++++++ tests/unit/fixtures/credentials.py | 13 ++ tests/unit/test_behavior_flags.py | 17 +- 7 files changed, 227 insertions(+), 231 deletions(-) create mode 100644 tests/unit/fixtures/__init__.py create mode 100644 tests/unit/fixtures/adapter.py create mode 100644 tests/unit/fixtures/connection_manager.py create mode 100644 tests/unit/fixtures/credentials.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index caf34209..0f2a03f7 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -38,7 +38,7 @@ repos: - --max-line-length=99 - --select=E,F,W - --ignore=E203,E501,E704,E741,W503,W504 - - --per-file-ignores=*/__init__.py:F401 + - --per-file-ignores=*/__init__.py:F401,*/conftest.py:F401 - repo: https://github.com/pre-commit/mirrors-mypy rev: v1.9.0 diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 7634f625..346634df 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -1,220 +1 @@ -from contextlib import contextmanager -from multiprocessing import get_context -from types import SimpleNamespace -from typing import Any, ContextManager, Dict, List, Optional, Tuple - -import agate -from dbt_common.behavior_flags import BehaviorFlag -import pytest - -from dbt.adapters.base.column import Column -from dbt.adapters.base.connections import BaseConnectionManager -from dbt.adapters.base.impl import BaseAdapter -from dbt.adapters.base.relation import BaseRelation -from dbt.adapters.contracts.connection import ( - AdapterRequiredConfig, - AdapterResponse, - Connection, - ConnectionState, - Credentials, - QueryComment, -) - - -@pytest.fixture -def adapter(config, existing_relations, behavior_flags) -> BaseAdapter: - - adapter = BaseAdapterStub(config, get_context("spawn")) - - adapter.behavior = behavior_flags - - for relation in existing_relations: - adapter.cache_added(BaseRelation.create(**relation)) - - return adapter - - -@pytest.fixture -def config(config_extra) -> AdapterRequiredConfig: - raw_config = { - "credentials": CredentialsStub("test_database", "test_schema"), - "profile_name": "test_profile", - "target_name": "test_target", - "threads": 4, - "project_name": "test_project", - "query_comment": QueryComment(), - "cli_vars": {}, - "target_path": "path/to/nowhere", - "log_cache_events": False, - } - raw_config.update(config_extra) - return SimpleNamespace(**raw_config) - - -@pytest.fixture -def config_extra() -> Dict[str, Any]: - return {} - - -@pytest.fixture -def existing_relations() -> List[Dict[str, str]]: - return [] - - -@pytest.fixture -def behavior_flags() -> List[BehaviorFlag]: - return [] - - -class CredentialsStub(Credentials): - """ - A stub for a database credentials that does not connect to a database - """ - - def type(self) -> str: - return "test" - - def _connection_keys(self): - return {"database": self.database, "schema": self.schema} - - -class ConnectionManagerStub(BaseConnectionManager): - """ - A stub for a connection manager that does not connect to a database - """ - - raised_exceptions: List[Exception] - - @contextmanager - def exception_handler(self, sql: str) -> ContextManager: # type: ignore - # catch all exceptions and put them on this class for inspection in tests - try: - yield - except Exception as exc: - self.raised_exceptions.append(exc) - finally: - pass - - def cancel_open(self) -> Optional[List[str]]: - names = [] - for connection in self.thread_connections.values(): - if connection.state == ConnectionState.OPEN: - connection.state = ConnectionState.CLOSED - if name := connection.name: - names.append(name) - return names - - @classmethod - def open(cls, connection: Connection) -> Connection: - # there's no database, so just change the state - connection.state = ConnectionState.OPEN - return connection - - def begin(self) -> None: - # there's no database, so there are no transactions - pass - - def commit(self) -> None: - # there's no database, so there are no transactions - pass - - def execute( - self, - sql: str, - auto_begin: bool = False, - fetch: bool = False, - limit: Optional[int] = None, - ) -> Tuple[AdapterResponse, agate.Table]: - # there's no database, so just return the sql - return AdapterResponse(_message="", code=sql), agate.Table([]) - - -class BaseAdapterStub(BaseAdapter): - """ - A stub for an adapter that uses the cache as the database - """ - - ConnectionManager = ConnectionManagerStub - - ### - # Abstract methods for database-specific values, attributes, and types - ### - @classmethod - def date_function(cls) -> str: - return "date_function" - - @classmethod - def is_cancelable(cls) -> bool: - return False - - def list_schemas(self, database: str) -> List[str]: - return list(self.cache.schemas) - - ### - # Abstract methods about relations - ### - def drop_relation(self, relation: BaseRelation) -> None: - self.cache_dropped(relation) - - def truncate_relation(self, relation: BaseRelation) -> None: - self.cache_dropped(relation) - - def rename_relation(self, from_relation: BaseRelation, to_relation: BaseRelation) -> None: - self.cache_renamed(from_relation, to_relation) - - def get_columns_in_relation(self, relation: BaseRelation) -> List[Column]: - # there's no database, so these need to be added as kwargs in the existing_relations fixture - return relation.columns - - def expand_column_types(self, goal: BaseRelation, current: BaseRelation) -> None: - # there's no database, so these need to be added as kwargs in the existing_relations fixture - object.__setattr__(current, "columns", goal.columns) - - def list_relations_without_caching(self, schema_relation: BaseRelation) -> List[BaseRelation]: - # there's no database, so use the cache as the database - return self.cache.get_relations(schema_relation.database, schema_relation.schema) - - ### - # ODBC FUNCTIONS -- these should not need to change for every adapter, - # although some adapters may override them - ### - def create_schema(self, relation: BaseRelation): - # there's no database, this happens implicitly by adding a relation to the cache - pass - - def drop_schema(self, relation: BaseRelation): - for each_relation in self.cache.get_relations(relation.database, relation.schema): - self.cache_dropped(each_relation) - - @classmethod - def quote(cls, identifier: str) -> str: - quote_char = "" - return f"{quote_char}{identifier}{quote_char}" - - ### - # Conversions: These must be implemented by concrete implementations, for - # converting agate types into their sql equivalents. - ### - @classmethod - def convert_text_type(cls, agate_table: agate.Table, col_idx: int) -> str: - return "str" - - @classmethod - def convert_number_type(cls, agate_table: agate.Table, col_idx: int) -> str: - return "float" - - @classmethod - def convert_boolean_type(cls, agate_table: agate.Table, col_idx: int) -> str: - return "bool" - - @classmethod - def convert_datetime_type(cls, agate_table: agate.Table, col_idx: int) -> str: - return "datetime" - - @classmethod - def convert_date_type(cls, *args, **kwargs): - return "date" - - @classmethod - def convert_time_type(cls, *args, **kwargs): - return "time" +from tests.unit.fixtures import adapter, behavior_flags, config, flags diff --git a/tests/unit/fixtures/__init__.py b/tests/unit/fixtures/__init__.py new file mode 100644 index 00000000..78135a2c --- /dev/null +++ b/tests/unit/fixtures/__init__.py @@ -0,0 +1 @@ +from tests.unit.fixtures.adapter import adapter, behavior_flags, config, flags diff --git a/tests/unit/fixtures/adapter.py b/tests/unit/fixtures/adapter.py new file mode 100644 index 00000000..b59b0423 --- /dev/null +++ b/tests/unit/fixtures/adapter.py @@ -0,0 +1,146 @@ +from multiprocessing import get_context +from types import SimpleNamespace +from typing import Any, Dict, List + +import agate +from dbt_common.behavior_flags import BehaviorFlag +import pytest + +from dbt.adapters.base.column import Column +from dbt.adapters.base.impl import BaseAdapter +from dbt.adapters.base.relation import BaseRelation +from dbt.adapters.contracts.connection import AdapterRequiredConfig, QueryComment + +from tests.unit.fixtures.connection_manager import ConnectionManagerStub +from tests.unit.fixtures.credentials import CredentialsStub + + +@pytest.fixture +def adapter(config, behavior_flags) -> BaseAdapter: + + class BaseAdapterStub(BaseAdapter): + """ + A stub for an adapter that uses the cache as the database + """ + + ConnectionManager = ConnectionManagerStub + + @property + def _behavior_flags(self) -> List[BehaviorFlag]: + return behavior_flags + + ### + # Abstract methods for database-specific values, attributes, and types + ### + @classmethod + def date_function(cls) -> str: + return "date_function" + + @classmethod + def is_cancelable(cls) -> bool: + return False + + def list_schemas(self, database: str) -> List[str]: + return list(self.cache.schemas) + + ### + # Abstract methods about relations + ### + def drop_relation(self, relation: BaseRelation) -> None: + self.cache_dropped(relation) + + def truncate_relation(self, relation: BaseRelation) -> None: + self.cache_dropped(relation) + + def rename_relation(self, from_relation: BaseRelation, to_relation: BaseRelation) -> None: + self.cache_renamed(from_relation, to_relation) + + def get_columns_in_relation(self, relation: BaseRelation) -> List[Column]: + # there's no database, so these need to be added as kwargs in the existing_relations fixture + return relation.columns + + def expand_column_types(self, goal: BaseRelation, current: BaseRelation) -> None: + # there's no database, so these need to be added as kwargs in the existing_relations fixture + object.__setattr__(current, "columns", goal.columns) + + def list_relations_without_caching( + self, schema_relation: BaseRelation + ) -> List[BaseRelation]: + # there's no database, so use the cache as the database + return self.cache.get_relations(schema_relation.database, schema_relation.schema) + + ### + # ODBC FUNCTIONS -- these should not need to change for every adapter, + # although some adapters may override them + ### + def create_schema(self, relation: BaseRelation): + # there's no database, this happens implicitly by adding a relation to the cache + pass + + def drop_schema(self, relation: BaseRelation): + for each_relation in self.cache.get_relations(relation.database, relation.schema): + self.cache_dropped(each_relation) + + @classmethod + def quote(cls, identifier: str) -> str: + quote_char = "" + return f"{quote_char}{identifier}{quote_char}" + + ### + # Conversions: These must be implemented by concrete implementations, for + # converting agate types into their sql equivalents. + ### + @classmethod + def convert_text_type(cls, agate_table: agate.Table, col_idx: int) -> str: + return "str" + + @classmethod + def convert_number_type(cls, agate_table: agate.Table, col_idx: int) -> str: + return "float" + + @classmethod + def convert_boolean_type(cls, agate_table: agate.Table, col_idx: int) -> str: + return "bool" + + @classmethod + def convert_datetime_type(cls, agate_table: agate.Table, col_idx: int) -> str: + return "datetime" + + @classmethod + def convert_date_type(cls, *args, **kwargs): + return "date" + + @classmethod + def convert_time_type(cls, *args, **kwargs): + return "time" + + return BaseAdapterStub(config, get_context("spawn")) + + +@pytest.fixture +def config(flags) -> AdapterRequiredConfig: + raw_config = { + "credentials": CredentialsStub("test_database", "test_schema"), + "profile_name": "test_profile", + "target_name": "test_target", + "threads": 4, + "project_name": "test_project", + "query_comment": QueryComment(), + "cli_vars": {}, + "target_path": "path/to/nowhere", + "log_cache_events": False, + "flags": flags, + } + return SimpleNamespace(**raw_config) + + +@pytest.fixture +def flags() -> Dict[str, Any]: + # this is the flags collection in dbt_project.yaml + return {} + + +@pytest.fixture +def behavior_flags() -> List[BehaviorFlag]: + # this is the collection of behavior flags for a specific adapter + return [] diff --git a/tests/unit/fixtures/connection_manager.py b/tests/unit/fixtures/connection_manager.py new file mode 100644 index 00000000..8b353fbe --- /dev/null +++ b/tests/unit/fixtures/connection_manager.py @@ -0,0 +1,58 @@ +from contextlib import contextmanager +from typing import ContextManager, List, Optional, Tuple + +import agate + +from dbt.adapters.base.connections import BaseConnectionManager +from dbt.adapters.contracts.connection import AdapterResponse, Connection, ConnectionState + + +class ConnectionManagerStub(BaseConnectionManager): + """ + A stub for a connection manager that does not connect to a database + """ + + raised_exceptions: List[Exception] + + @contextmanager + def exception_handler(self, sql: str) -> ContextManager: # type: ignore + # catch all exceptions and put them on this class for inspection in tests + try: + yield + except Exception as exc: + self.raised_exceptions.append(exc) + finally: + pass + + def cancel_open(self) -> Optional[List[str]]: + names = [] + for connection in self.thread_connections.values(): + if connection.state == ConnectionState.OPEN: + connection.state = ConnectionState.CLOSED + if name := connection.name: + names.append(name) + return names + + @classmethod + def open(cls, connection: Connection) -> Connection: + # there's no database, so just change the state + connection.state = ConnectionState.OPEN + return connection + + def begin(self) -> None: + # there's no database, so there are no transactions + pass + + def commit(self) -> None: + # there's no database, so there are no transactions + pass + + def execute( + self, + sql: str, + auto_begin: bool = False, + fetch: bool = False, + limit: Optional[int] = None, + ) -> Tuple[AdapterResponse, agate.Table]: + # there's no database, so just return the sql + return AdapterResponse(_message="", code=sql), agate.Table([]) diff --git a/tests/unit/fixtures/credentials.py b/tests/unit/fixtures/credentials.py new file mode 100644 index 00000000..88817f6b --- /dev/null +++ b/tests/unit/fixtures/credentials.py @@ -0,0 +1,13 @@ +from dbt.adapters.contracts.connection import Credentials + + +class CredentialsStub(Credentials): + """ + A stub for a database credentials that does not connect to a database + """ + + def type(self) -> str: + return "test" + + def _connection_keys(self): + return {"database": self.database, "schema": self.schema} diff --git a/tests/unit/test_behavior_flags.py b/tests/unit/test_behavior_flags.py index b43e05b3..0ae1a021 100644 --- a/tests/unit/test_behavior_flags.py +++ b/tests/unit/test_behavior_flags.py @@ -6,17 +6,14 @@ @pytest.fixture -def config_extra() -> Dict[str, Any]: - config = { - "flags": { - "unregistered_flag": True, - "default_false_user_false_flag": False, - "default_false_user_true_flag": True, - "default_true_user_false_flag": False, - "default_true_user_true_flag": True, - } +def flags() -> Dict[str, Any]: + return { + "unregistered_flag": True, + "default_false_user_false_flag": False, + "default_false_user_true_flag": True, + "default_true_user_false_flag": False, + "default_true_user_true_flag": True, } - return config @pytest.fixture From e5766f2afcbfe7e987ae50a02fdc859f4febc56f Mon Sep 17 00:00:00 2001 From: Mike Alfare Date: Fri, 6 Sep 2024 15:23:03 -0400 Subject: [PATCH 17/17] point back to main --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index 8466eed5..e794781c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,7 +54,7 @@ include = ["dbt/adapters", "dbt/include", "dbt/__init__.py"] [tool.hatch.envs.default] dependencies = [ - "dbt_common @ git+https://github.com/dbt-labs/dbt-common.git@behavior-flags", + "dbt_common @ git+https://github.com/dbt-labs/dbt-common.git", 'pre-commit==3.7.0;python_version>="3.9"', 'pre-commit==3.5.0;python_version=="3.8"', "pytest",