UX improvements to model versions (#7435)

* Latest version should use un-suffixed alias * Latest version can be in un-suffixed file * FYI when unpinned ref to model with prerelease version * [WIP] Nicer error if versioned ref to unversioned model * Revert "Latest version should use un-suffixed alias" This reverts commit 3616c52. * Revert "[WIP] Nicer error if versioned ref to unversioned model" This reverts commit c9ae4af. * Define real event for UnpinnedRefNewVersionAvailable * Update pp test for implicit unsuffixed defined_in * Add changelog entry * Fix unit test * marky feedback * Add test case for UnpinnedRefNewVersionAvailable event (cherry picked from commit d53bb37)
dbt-labs · Apr 25, 2023 · f67362d · f67362d
1 parent 77867d7
commit f67362d
Show file tree

Hide file tree

Showing 10 changed files with 546 additions and 442 deletions.
diff --git a/.changes/unreleased/Under the Hood-20230424-135300.yaml b/.changes/unreleased/Under the Hood-20230424-135300.yaml
@@ -0,0 +1,8 @@
+kind: Under the Hood
+body: 'Small UX improvements to model versions: Support defining latest_version in
+  unsuffixed file by default. Notify on unpinned ref when a prerelease version is
+  available. '
+time: 2023-04-24T13:53:00.484916+02:00
+custom:
+  Author: jtcohen6
+  Issue: "7443"
diff --git a/core/dbt/contracts/graph/manifest.py b/core/dbt/contracts/graph/manifest.py
@@ -36,7 +36,7 @@
     ResultNode,
     BaseNode,
 )
-from dbt.contracts.graph.unparsed import SourcePatch, NodeVersion
+from dbt.contracts.graph.unparsed import SourcePatch, NodeVersion, UnparsedVersion
 from dbt.contracts.graph.manifest_upgrade import upgrade_manifest_json
 from dbt.contracts.files import SourceFile, SchemaSourceFile, FileHash, AnySourceFile
 from dbt.contracts.util import BaseArtifactMetadata, SourceKey, ArtifactMixin, schema_version
@@ -49,7 +49,8 @@
 )
 from dbt.helper_types import PathSet
 from dbt.events.functions import fire_event
-from dbt.events.types import MergedFromState
+from dbt.events.types import MergedFromState, UnpinnedRefNewVersionAvailable
+from dbt.events.contextvars import get_node_info
 from dbt.node_types import NodeType
 from dbt.flags import get_flags, MP_CONTEXT
 from dbt import tracking
@@ -169,7 +170,32 @@ def find(
     ):
         unique_id = self.get_unique_id(key, package, version)
         if unique_id is not None:
-            return self.perform_lookup(unique_id, manifest)
+            node = self.perform_lookup(unique_id, manifest)
+            # If this is an unpinned ref (no 'version' arg was passed),
+            # AND this is a versioned node,
+            # AND this ref is being resolved at runtime -- get_node_info != {}
+            if version is None and node.is_versioned and get_node_info():
+                # Check to see if newer versions are available, and log an "FYI" if so
+                max_version: UnparsedVersion = max(
+                    [
+                        UnparsedVersion(v.version)
+                        for v in manifest.nodes.values()
+                        if v.name == node.name and v.version is not None
+                    ]
+                )
+                assert node.latest_version  # for mypy, whenever i may find it
+                if max_version > UnparsedVersion(node.latest_version):
+                    fire_event(
+                        UnpinnedRefNewVersionAvailable(
+                            node_info=get_node_info(),
+                            ref_node_name=node.name,
+                            ref_node_package=node.package_name,
+                            ref_node_version=str(node.version),
+                            ref_max_version=str(max_version.v),
+                        )
+                    )
+
+            return node
         return None
 
     def add_node(self, node: ManifestNode):

diff --git a/core/dbt/events/types.proto b/core/dbt/events/types.proto
@@ -1145,7 +1145,7 @@ message JinjaLogInfo {
 
 message  JinjaLogInfoMsg {
     EventInfo info = 1;
-     JinjaLogInfo data = 2;
+    JinjaLogInfo data = 2;
 }
 
 // I063
@@ -1159,6 +1159,20 @@ message JinjaLogDebugMsg {
     JinjaLogDebug data = 2;
 }
 
+// I064
+message UnpinnedRefNewVersionAvailable {
+    NodeInfo node_info = 1;
+    string ref_node_name  = 2;
+    string ref_node_package = 3;
+    string ref_node_version = 4;
+    string ref_max_version = 5;
+}
+
+message UnpinnedRefNewVersionAvailableMsg {
+    EventInfo info = 1;
+    UnpinnedRefNewVersionAvailable data = 2;
+}
+
 // M - Deps generation
 
 // M001

diff --git a/core/dbt/events/types.py b/core/dbt/events/types.py
@@ -1118,6 +1118,23 @@ def message(self) -> str:
         return self.msg
 
 
+class UnpinnedRefNewVersionAvailable(InfoLevel):
+    def code(self):
+        return "I064"
+
+    def message(self) -> str:
+        msg = (
+            f"While compiling '{self.node_info.node_name}':\n"
+            f"Found an unpinned reference to versioned model '{self.ref_node_name}' in project '{self.ref_node_package}'.\n"
+            f"Resolving to latest version: {self.ref_node_name}.v{self.ref_node_version}\n"
+            f"A prerelease version {self.ref_max_version} is available. It has not yet been marked 'latest' by its maintainer.\n"
+            f"When that happens, this reference will resolve to {self.ref_node_name}.v{self.ref_max_version} instead.\n\n"
+            f"  Try out v{self.ref_max_version}: {{{{ ref('{self.ref_node_package}', '{self.ref_node_name}', v='{self.ref_max_version}') }}}}\n"
+            f"  Pin to  v{self.ref_node_version}: {{{{ ref('{self.ref_node_package}', '{self.ref_node_name}', v='{self.ref_node_version}') }}}}\n"
+        )
+        return msg
+
+
 # =======================================================
 # M - Deps generation
 # =======================================================

diff --git a/core/dbt/events/types_pb2.py b/core/dbt/events/types_pb2.py
diff --git a/core/dbt/parser/schemas.py b/core/dbt/parser/schemas.py
@@ -1082,6 +1082,13 @@ def parse_patch(self, block: TargetBlock[UnparsedModelUpdate], refs: ParserRef)
 
                 versioned_model_node = None
                 add_node_nofile_fn: Callable
+
+                # If this is the latest version, it's allowed to define itself in a model file name that doesn't have a suffix
+                if versioned_model_unique_id is None and unparsed_version.v == latest_version:
+                    versioned_model_unique_id = self.manifest.ref_lookup.get_unique_id(
+                        block.name, None, None
+                    )
+
                 if versioned_model_unique_id is None:
                     # Node might be disabled. Following call returns list of matching disabled nodes
                     found_nodes = self.manifest.disabled_lookup.find(versioned_model_name, None)

diff --git a/test/unit/utils.py b/test/unit/utils.py
@@ -346,6 +346,7 @@ def MockNode(package, name, resource_type=None, **kwargs):
         **kwargs,
     )
     node.name = name
+    node.is_versioned = resource_type is NodeType.Model and version is not None
     return node
 
 

diff --git a/tests/functional/partial_parsing/fixtures.py b/tests/functional/partial_parsing/fixtures.py
@@ -881,10 +881,20 @@
       description: "The first model"
       versions:
         - v: 1
-          defined_in: model_one
         - v: 2
 """
 
+models_versions_defined_in_schema_yml = """
+
+models:
+    - name: model_one
+      description: "The first model"
+      versions:
+        - v: 1
+        - v: 2
+          defined_in: model_one_different
+"""
+
 models_versions_updated_schema_yml = """
 
 models:
@@ -893,8 +903,8 @@
       description: "The first model"
       versions:
         - v: 1
-          defined_in: model_one
         - v: 2
+          defined_in: model_one_different
 """
 
 my_macro_sql = """

diff --git a/tests/functional/partial_parsing/test_partial_parsing.py b/tests/functional/partial_parsing/test_partial_parsing.py
@@ -9,6 +9,7 @@
     models_schema2_yml,
     models_schema2b_yml,
     models_versions_schema_yml,
+    models_versions_defined_in_schema_yml,
     models_versions_updated_schema_yml,
     model_three_sql,
     model_three_modified_sql,
@@ -302,8 +303,8 @@ class TestVersionedModels:
     @pytest.fixture(scope="class")
     def models(self):
         return {
+            "model_one_v1.sql": model_one_sql,
             "model_one.sql": model_one_sql,
-            "model_one_v2.sql": model_one_sql,
             "model_one_downstream.sql": model_four2_sql,
             "schema.yml": models_versions_schema_yml,
         }
@@ -321,11 +322,22 @@ def test_pp_versioned_models(self, project):
         model_one_downstream_node = manifest.nodes["model.test.model_one_downstream"]
         assert model_one_downstream_node.depends_on.nodes == ["model.test.model_one.v2"]
 
+        # update schema.yml block - model_one is now 'defined_in: model_one_different'
+        rm_file(project.project_root, "models", "model_one.sql")
+        write_file(model_one_sql, project.project_root, "models", "model_one_different.sql")
+        write_file(
+            models_versions_defined_in_schema_yml, project.project_root, "models", "schema.yml"
+        )
+        results = run_dbt(["--partial-parse", "run"])
+        assert len(results) == 3
+
         # update versions schema.yml block - latest_version from 2 to 1
         write_file(
             models_versions_updated_schema_yml, project.project_root, "models", "schema.yml"
         )
-        results = run_dbt(["--partial-parse", "run"])
+        results, log_output = run_dbt_and_capture(
+            ["--partial-parse", "--log-format", "json", "run"]
+        )
         assert len(results) == 3
 
         manifest = get_manifest(project.project_root)
@@ -336,9 +348,11 @@ def test_pp_versioned_models(self, project):
         # assert unpinned ref points to latest version
         model_one_downstream_node = manifest.nodes["model.test.model_one_downstream"]
         assert model_one_downstream_node.depends_on.nodes == ["model.test.model_one.v1"]
+        # assert unpinned ref to latest-not-max version yields an "FYI" info-level log
+        assert "UnpinnedRefNewVersionAvailable" in log_output
 
         # update versioned model
-        write_file(model_two_sql, project.project_root, "models", "model_one_v2.sql")
+        write_file(model_two_sql, project.project_root, "models", "model_one_different.sql")
         results = run_dbt(["--partial-parse", "run"])
         assert len(results) == 3
 

diff --git a/tests/unit/test_events.py b/tests/unit/test_events.py
@@ -228,6 +228,9 @@ def test_event_codes(self):
     types.JinjaLogWarning(),
     types.JinjaLogInfo(msg=""),
     types.JinjaLogDebug(msg=""),
+    types.UnpinnedRefNewVersionAvailable(
+        ref_node_name="", ref_node_package="", ref_node_version="", ref_max_version=""
+    ),
     # M - Deps generation ======================
     types.GitSparseCheckoutSubdirectory(subdir=""),
     types.GitProgressCheckoutRevision(revision=""),