Feature: state:modified.macros (#3559) (#3736)

* First cut at state:modified for macro changes

* First cut at state:modified subselectors

* Update test_graph_selector_methods

* Fix flake8

* Fix mypy

* Update 062_defer_state_test/test_modified_state

* PR feedback. Update changelog

CHANGELOG.md

@@ -1,5 +1,8 @@
 ## dbt 0.21.0 (Release TBD)
 
+### Features
+- Capture changes to macros in `state:modified`. Introduce new `state:` sub-selectors: `modified.body`, `modified.configs`, `modified.persisted_descriptions`, `modified.relation`, `modified.macros` ([#2704](https://github.com/dbt-labs/dbt/issues/2704), [#3278](https://github.com/dbt-labs/dbt/issues/3278), [#3559](https://github.com/dbt-labs/dbt/issues/3559))
+
 ### Fixes
 - Fix for RPC requests that raise a RecursionError when serializing Undefined values as JSON ([#3464](https://github.com/dbt-labs/dbt/issues/3464), [#3687](https://github.com/dbt-labs/dbt/pull/3687))
 

core/dbt/contracts/graph/parsed.py

@@ -692,7 +692,7 @@ def depends_on_nodes(self):
 
     @property
     def depends_on(self):
-        return {'nodes': []}
+        return DependsOn(macros=[], nodes=[])
 
     @property
     def refs(self):

core/dbt/graph/selector_methods.py

@@ -22,13 +22,11 @@
     ParsedSourceDefinition,
 )
 from dbt.contracts.state import PreviousState
-from dbt.logger import GLOBAL_LOGGER as logger
 from dbt.exceptions import (
     InternalException,
     RuntimeException,
 )
 from dbt.node_types import NodeType
-from dbt.ui import warning_tag
 
 
 SELECTOR_GLOB = '*'
@@ -381,7 +379,7 @@ def search(
 class StateSelectorMethod(SelectorMethod):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
-        self.macros_were_modified: Optional[List[str]] = None
+        self.modified_macros: Optional[List[str]] = None
 
     def _macros_modified(self) -> List[str]:
         # we checked in the caller!
@@ -394,44 +392,74 @@ def _macros_modified(self) -> List[str]:
 
         modified = []
         for uid, macro in new_macros.items():
-            name = f'{macro.package_name}.{macro.name}'
             if uid in old_macros:
                 old_macro = old_macros[uid]
                 if macro.macro_sql != old_macro.macro_sql:
-                    modified.append(f'{name} changed')
+                    modified.append(uid)
             else:
-                modified.append(f'{name} added')
+                modified.append(uid)
 
         for uid, macro in old_macros.items():
             if uid not in new_macros:
-                modified.append(f'{macro.package_name}.{macro.name} removed')
+                modified.append(uid)
+
+        return modified
+
+    def recursively_check_macros_modified(self, node):
+        # check if there are any changes in macros the first time
+        if self.modified_macros is None:
+            self.modified_macros = self._macros_modified()
+
+        # loop through all macros that this node depends on
+        for macro_uid in node.depends_on.macros:
+            # is this macro one of the modified macros?
+            if macro_uid in self.modified_macros:
+                return True
+            # if not, and this macro depends on other macros, keep looping
+            macro = self.manifest.macros[macro_uid]
+            if len(macro.depends_on.macros) > 0:
+                return self.recursively_check_macros_modified(macro)
+            else:
+                return False
+        return False
 
-        return modified[:3]
+    def check_modified(self, old: Optional[SelectorTarget], new: SelectorTarget) -> bool:
+        different_contents = not new.same_contents(old)  # type: ignore
+        upstream_macro_change = self.recursively_check_macros_modified(new)
+        return different_contents or upstream_macro_change
 
-    def check_modified(
-        self,
-        old: Optional[SelectorTarget],
-        new: SelectorTarget,
+    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:
-        # check if there are any changes in macros, if so, log a warning the
-        # first time
-        if self.macros_were_modified is None:
-            self.macros_were_modified = self._macros_modified()
-            if self.macros_were_modified:
-                log_str = ', '.join(self.macros_were_modified)
-                logger.warning(warning_tag(
-                    f'During a state comparison, dbt detected a change in '
-                    f'macros. This will not be marked as a modification. Some '
-                    f'macros: {log_str}'
-                ))
-
-        return not new.same_contents(old)  # type: ignore
-
-    def check_new(
-        self,
-        old: Optional[SelectorTarget],
-        new: SelectorTarget,
+        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.recursively_check_macros_modified(new)
+
+    def check_new(self, old: Optional[SelectorTarget], new: SelectorTarget) -> bool:
         return old is None
 
     def search(
@@ -443,8 +471,15 @@ def search(
             )
 
         state_checks = {
+            # 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,
-            'new': self.check_new,
+            '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.macros': self.check_modified_macros,
         }
         if selector in state_checks:
             checker = state_checks[selector]

test/integration/062_defer_state_test/changed_models/table_model.sql

@@ -1,2 +1,5 @@
 {{ config(materialized='table') }}
 select * from {{ ref('ephemeral_model') }}
+
+-- establish a macro dependency to trigger state:modified.macros
+-- depends on: {{ my_macro() }}

test/integration/062_defer_state_test/changed_models_bad/table_model.sql

@@ -1,2 +1,5 @@
 {{ config(materialized='table') }}
 select * from {{ ref('ephemeral_model') }}
+
+-- establish a macro dependency to trigger state:modified.macros
+-- depends on: {{ my_macro() }}

test/integration/062_defer_state_test/models/table_model.sql

@@ -1,2 +1,5 @@
 {{ config(materialized='table') }}
 select * from {{ ref('ephemeral_model') }}
+
+-- establish a macro dependency to trigger state:modified.macros
+-- depends on: {{ my_macro() }}

test/integration/062_defer_state_test/test_modified_state.py

@@ -166,7 +166,6 @@ def test_postgres_new_macro(self):
 
         results, stdout = self.run_dbt_and_capture(['run', '--models', 'state:modified', '--state', './state'], strict=False)
         assert len(results) == 0
-        assert 'detected a change in macros' in stdout
 
         os.remove('macros/second_macro.sql')
         # add a new macro to the existing file
@@ -175,7 +174,6 @@ def test_postgres_new_macro(self):
 
         results, stdout = self.run_dbt_and_capture(['run', '--models', 'state:modified', '--state', './state'], strict=False)
         assert len(results) == 0
-        assert 'detected a change in macros' in stdout
 
     @use_profile('postgres')
     def test_postgres_changed_macro_contents(self):
@@ -192,9 +190,9 @@ def test_postgres_changed_macro_contents(self):
             fp.write('{% endmacro %}')
             fp.write(newline)
 
+        # table_model calls this macro
         results, stdout = self.run_dbt_and_capture(['run', '--models', 'state:modified', '--state', './state'], strict=False)
-        assert len(results) == 0
-        assert 'detected a change in macros' in stdout
+        assert len(results) == 1
 
     @use_profile('postgres')
     def test_postgres_changed_exposure(self):