From 0400818b60e5c8dd3aeb38c5f8ec505a2b8d9dd6 Mon Sep 17 00:00:00 2001 From: ChenyuLi Date: Thu, 27 Jan 2022 11:39:39 -0500 Subject: [PATCH 1/4] fix comparision for new model/body --- core/dbt/graph/selector_methods.py | 54 +++++++++--------------- test/unit/test_graph_selector_methods.py | 2 + 2 files changed, 21 insertions(+), 35 deletions(-) diff --git a/core/dbt/graph/selector_methods.py b/core/dbt/graph/selector_methods.py index 8df44144486..e5c20f559ad 100644 --- a/core/dbt/graph/selector_methods.py +++ b/core/dbt/graph/selector_methods.py @@ -1,7 +1,7 @@ import abc from itertools import chain from pathlib import Path -from typing import Set, List, Dict, Iterator, Tuple, Any, Union, Type, Optional +from typing import Set, List, Dict, Iterator, Tuple, Any, Union, Type, Optional, Callable from dbt.dataclass_schema import StrEnum @@ -478,42 +478,26 @@ def check_macros_modified(self, node): previous_macros = [] return self.recursively_check_macros_modified(node, previous_macros) - def check_modified(self, old: Optional[SelectorTarget], new: SelectorTarget) -> bool: + # TODO check modifed_content and check_modified macro seems a bit redundent + def check_modified_content(self, old: Optional[SelectorTarget], new: SelectorTarget) -> bool: different_contents = not new.same_contents(old) # type: ignore upstream_macro_change = self.check_macros_modified(new) return different_contents or upstream_macro_change - def check_modified_body(self, old: Optional[SelectorTarget], new: SelectorTarget) -> bool: - if hasattr(new, "same_body"): - return not new.same_body(old) # type: ignore - else: - return False - - def check_modified_configs(self, old: Optional[SelectorTarget], new: SelectorTarget) -> bool: - if hasattr(new, "same_config"): - return not new.same_config(old) # type: ignore - else: - return False - - def check_modified_persisted_descriptions( - self, old: Optional[SelectorTarget], new: SelectorTarget - ) -> bool: - if hasattr(new, "same_persisted_description"): - return not new.same_persisted_description(old) # type: ignore - else: - return False - - def check_modified_relation( - self, old: Optional[SelectorTarget], new: SelectorTarget - ) -> bool: - if hasattr(new, "same_database_representation"): - return not new.same_database_representation(old) # type: ignore - else: - return False - def check_modified_macros(self, _, new: SelectorTarget) -> bool: return self.check_macros_modified(new) + @staticmethod + def check_modified_factory(compare_method: str) -> Callable: + + def check_modified_things(old: Optional[SelectorTarget], new: SelectorTarget,) -> bool: + if hasattr(new, compare_method): + # when old body does not exist or old and new are not the same + return not old or not getattr(new, compare_method)(old) # type: ignore + else: + return False + return check_modified_things + def check_new(self, old: Optional[SelectorTarget], new: SelectorTarget) -> bool: return old is None @@ -529,11 +513,11 @@ def search( # it's new if there is no old version 'new': lambda old, _: old is None, # use methods defined above to compare properties of old + new - 'modified': self.check_modified, - 'modified.body': self.check_modified_body, - 'modified.configs': self.check_modified_configs, - 'modified.persisted_descriptions': self.check_modified_persisted_descriptions, - 'modified.relation': self.check_modified_relation, + 'modified': self.check_modified_content, + 'modified.body': self.check_modified_factory('same_body'), + 'modified.configs': self.check_modified_factory('same_config'), + 'modified.persisted_descriptions': self.check_modified_factory('same_persisted_description'), + 'modified.relation': self.check_modified_factory('same_database_representation'), 'modified.macros': self.check_modified_macros, } if selector in state_checks: diff --git a/test/unit/test_graph_selector_methods.py b/test/unit/test_graph_selector_methods.py index 0bd57015bb3..d8dd01ec237 100644 --- a/test/unit/test_graph_selector_methods.py +++ b/test/unit/test_graph_selector_methods.py @@ -835,6 +835,8 @@ def test_select_state_added_model(manifest, previous_state): manifest, method, 'modified') == {'another_model'} assert search_manifest_using_method( manifest, method, 'new') == {'another_model'} + assert search_manifest_using_method( + manifest, method, 'modified.body') == {'another_model'} def test_select_state_changed_model_sql(manifest, previous_state, view_model): From 48eb739828ce3644715386c0b08e993ceff0afde Mon Sep 17 00:00:00 2001 From: ChenyuLi Date: Thu, 27 Jan 2022 11:50:09 -0500 Subject: [PATCH 2/4] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92431bb811d..6d282a55ccc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ ### Fixes - Projects created using `dbt init` now have the correct `seeds` directory created (instead of `data`) ([#4588](https://github.com/dbt-labs/dbt-core/issues/4588), [#4599](https://github.com/dbt-labs/dbt-core/pull/4589)) - Don't require a profile for dbt deps and clean commands ([#4554](https://github.com/dbt-labs/dbt-core/issues/4554), [#4610](https://github.com/dbt-labs/dbt-core/pull/4610)) +- Select modified.body works correctly when new model added([#4570](https://github.com/dbt-labs/dbt-core/issues/4570), [#4631](https://github.com/dbt-labs/dbt-core/pull/4631)) ## dbt-core 1.0.1 (January 03, 2022) From fde85420556e27d082c9d273effaa8db2f770a76 Mon Sep 17 00:00:00 2001 From: ChenyuLi Date: Thu, 27 Jan 2022 11:59:01 -0500 Subject: [PATCH 3/4] fix style --- core/dbt/graph/selector_methods.py | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/core/dbt/graph/selector_methods.py b/core/dbt/graph/selector_methods.py index e5c20f559ad..dd221be4693 100644 --- a/core/dbt/graph/selector_methods.py +++ b/core/dbt/graph/selector_methods.py @@ -489,7 +489,7 @@ def check_modified_macros(self, _, new: SelectorTarget) -> bool: @staticmethod def check_modified_factory(compare_method: str) -> Callable: - + # get a function that compares two selector target based on compare method provided def check_modified_things(old: Optional[SelectorTarget], new: SelectorTarget,) -> bool: if hasattr(new, compare_method): # when old body does not exist or old and new are not the same @@ -511,14 +511,21 @@ def search( state_checks = { # it's new if there is no old version - 'new': lambda old, _: old is None, + 'new': + lambda old, _: old is None, # use methods defined above to compare properties of old + new - 'modified': self.check_modified_content, - 'modified.body': self.check_modified_factory('same_body'), - 'modified.configs': self.check_modified_factory('same_config'), - 'modified.persisted_descriptions': self.check_modified_factory('same_persisted_description'), - 'modified.relation': self.check_modified_factory('same_database_representation'), - 'modified.macros': self.check_modified_macros, + 'modified': + self.check_modified_content, + 'modified.body': + self.check_modified_factory('same_body'), + 'modified.configs': + self.check_modified_factory('same_config'), + 'modified.persisted_descriptions': + self.check_modified_factory('same_persisted_description'), + 'modified.relation': + self.check_modified_factory('same_database_representation'), + 'modified.macros': + self.check_modified_macros, } if selector in state_checks: checker = state_checks[selector] From 8a8dac30f8dd79bd2f0dab36cefff8c2138b3044 Mon Sep 17 00:00:00 2001 From: ChenyuLi Date: Thu, 27 Jan 2022 12:23:34 -0500 Subject: [PATCH 4/4] fix style --- core/dbt/graph/selector_methods.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/dbt/graph/selector_methods.py b/core/dbt/graph/selector_methods.py index dd221be4693..89b3befcb57 100644 --- a/core/dbt/graph/selector_methods.py +++ b/core/dbt/graph/selector_methods.py @@ -488,9 +488,11 @@ def check_modified_macros(self, _, new: SelectorTarget) -> bool: return self.check_macros_modified(new) @staticmethod - def check_modified_factory(compare_method: str) -> Callable: + def check_modified_factory( + compare_method: str + ) -> Callable[[Optional[SelectorTarget], SelectorTarget], bool]: # get a function that compares two selector target based on compare method provided - def check_modified_things(old: Optional[SelectorTarget], new: SelectorTarget,) -> bool: + def check_modified_things(old: Optional[SelectorTarget], new: SelectorTarget) -> bool: if hasattr(new, compare_method): # when old body does not exist or old and new are not the same return not old or not getattr(new, compare_method)(old) # type: ignore