Skip to content

Commit d53bb37

Browse files
authored
UX improvements to model versions (dbt-labs#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
1 parent 9874f9e commit d53bb37

File tree

10 files changed

+546
-442
lines changed

10 files changed

+546
-442
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
kind: Under the Hood
2+
body: 'Small UX improvements to model versions: Support defining latest_version in
3+
unsuffixed file by default. Notify on unpinned ref when a prerelease version is
4+
available. '
5+
time: 2023-04-24T13:53:00.484916+02:00
6+
custom:
7+
Author: jtcohen6
8+
Issue: "7443"

core/dbt/contracts/graph/manifest.py

+29-3
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636
ResultNode,
3737
BaseNode,
3838
)
39-
from dbt.contracts.graph.unparsed import SourcePatch, NodeVersion
39+
from dbt.contracts.graph.unparsed import SourcePatch, NodeVersion, UnparsedVersion
4040
from dbt.contracts.graph.manifest_upgrade import upgrade_manifest_json
4141
from dbt.contracts.files import SourceFile, SchemaSourceFile, FileHash, AnySourceFile
4242
from dbt.contracts.util import BaseArtifactMetadata, SourceKey, ArtifactMixin, schema_version
@@ -49,7 +49,8 @@
4949
)
5050
from dbt.helper_types import PathSet
5151
from dbt.events.functions import fire_event
52-
from dbt.events.types import MergedFromState
52+
from dbt.events.types import MergedFromState, UnpinnedRefNewVersionAvailable
53+
from dbt.events.contextvars import get_node_info
5354
from dbt.node_types import NodeType
5455
from dbt.flags import get_flags, MP_CONTEXT
5556
from dbt import tracking
@@ -169,7 +170,32 @@ def find(
169170
):
170171
unique_id = self.get_unique_id(key, package, version)
171172
if unique_id is not None:
172-
return self.perform_lookup(unique_id, manifest)
173+
node = self.perform_lookup(unique_id, manifest)
174+
# If this is an unpinned ref (no 'version' arg was passed),
175+
# AND this is a versioned node,
176+
# AND this ref is being resolved at runtime -- get_node_info != {}
177+
if version is None and node.is_versioned and get_node_info():
178+
# Check to see if newer versions are available, and log an "FYI" if so
179+
max_version: UnparsedVersion = max(
180+
[
181+
UnparsedVersion(v.version)
182+
for v in manifest.nodes.values()
183+
if v.name == node.name and v.version is not None
184+
]
185+
)
186+
assert node.latest_version # for mypy, whenever i may find it
187+
if max_version > UnparsedVersion(node.latest_version):
188+
fire_event(
189+
UnpinnedRefNewVersionAvailable(
190+
node_info=get_node_info(),
191+
ref_node_name=node.name,
192+
ref_node_package=node.package_name,
193+
ref_node_version=str(node.version),
194+
ref_max_version=str(max_version.v),
195+
)
196+
)
197+
198+
return node
173199
return None
174200

175201
def add_node(self, node: ManifestNode):

core/dbt/events/types.proto

+15-1
Original file line numberDiff line numberDiff line change
@@ -1145,7 +1145,7 @@ message JinjaLogInfo {
11451145

11461146
message JinjaLogInfoMsg {
11471147
EventInfo info = 1;
1148-
JinjaLogInfo data = 2;
1148+
JinjaLogInfo data = 2;
11491149
}
11501150

11511151
// I063
@@ -1159,6 +1159,20 @@ message JinjaLogDebugMsg {
11591159
JinjaLogDebug data = 2;
11601160
}
11611161

1162+
// I064
1163+
message UnpinnedRefNewVersionAvailable {
1164+
NodeInfo node_info = 1;
1165+
string ref_node_name = 2;
1166+
string ref_node_package = 3;
1167+
string ref_node_version = 4;
1168+
string ref_max_version = 5;
1169+
}
1170+
1171+
message UnpinnedRefNewVersionAvailableMsg {
1172+
EventInfo info = 1;
1173+
UnpinnedRefNewVersionAvailable data = 2;
1174+
}
1175+
11621176
// M - Deps generation
11631177

11641178
// M001

core/dbt/events/types.py

+17
Original file line numberDiff line numberDiff line change
@@ -1118,6 +1118,23 @@ def message(self) -> str:
11181118
return self.msg
11191119

11201120

1121+
class UnpinnedRefNewVersionAvailable(InfoLevel):
1122+
def code(self):
1123+
return "I064"
1124+
1125+
def message(self) -> str:
1126+
msg = (
1127+
f"While compiling '{self.node_info.node_name}':\n"
1128+
f"Found an unpinned reference to versioned model '{self.ref_node_name}' in project '{self.ref_node_package}'.\n"
1129+
f"Resolving to latest version: {self.ref_node_name}.v{self.ref_node_version}\n"
1130+
f"A prerelease version {self.ref_max_version} is available. It has not yet been marked 'latest' by its maintainer.\n"
1131+
f"When that happens, this reference will resolve to {self.ref_node_name}.v{self.ref_max_version} instead.\n\n"
1132+
f" Try out v{self.ref_max_version}: {{{{ ref('{self.ref_node_package}', '{self.ref_node_name}', v='{self.ref_max_version}') }}}}\n"
1133+
f" Pin to v{self.ref_node_version}: {{{{ ref('{self.ref_node_package}', '{self.ref_node_name}', v='{self.ref_node_version}') }}}}\n"
1134+
)
1135+
return msg
1136+
1137+
11211138
# =======================================================
11221139
# M - Deps generation
11231140
# =======================================================

core/dbt/events/types_pb2.py

+437-433
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

core/dbt/parser/schemas.py

+7
Original file line numberDiff line numberDiff line change
@@ -1082,6 +1082,13 @@ def parse_patch(self, block: TargetBlock[UnparsedModelUpdate], refs: ParserRef)
10821082

10831083
versioned_model_node = None
10841084
add_node_nofile_fn: Callable
1085+
1086+
# If this is the latest version, it's allowed to define itself in a model file name that doesn't have a suffix
1087+
if versioned_model_unique_id is None and unparsed_version.v == latest_version:
1088+
versioned_model_unique_id = self.manifest.ref_lookup.get_unique_id(
1089+
block.name, None, None
1090+
)
1091+
10851092
if versioned_model_unique_id is None:
10861093
# Node might be disabled. Following call returns list of matching disabled nodes
10871094
found_nodes = self.manifest.disabled_lookup.find(versioned_model_name, None)

test/unit/utils.py

+1
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,7 @@ def MockNode(package, name, resource_type=None, **kwargs):
346346
**kwargs,
347347
)
348348
node.name = name
349+
node.is_versioned = resource_type is NodeType.Model and version is not None
349350
return node
350351

351352

tests/functional/partial_parsing/fixtures.py

+12-2
Original file line numberDiff line numberDiff line change
@@ -886,10 +886,20 @@
886886
description: "The first model"
887887
versions:
888888
- v: 1
889-
defined_in: model_one
890889
- v: 2
891890
"""
892891

892+
models_versions_defined_in_schema_yml = """
893+
894+
models:
895+
- name: model_one
896+
description: "The first model"
897+
versions:
898+
- v: 1
899+
- v: 2
900+
defined_in: model_one_different
901+
"""
902+
893903
models_versions_updated_schema_yml = """
894904
895905
models:
@@ -898,8 +908,8 @@
898908
description: "The first model"
899909
versions:
900910
- v: 1
901-
defined_in: model_one
902911
- v: 2
912+
defined_in: model_one_different
903913
"""
904914

905915
my_macro_sql = """

tests/functional/partial_parsing/test_partial_parsing.py

+17-3
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
models_schema2_yml,
1010
models_schema2b_yml,
1111
models_versions_schema_yml,
12+
models_versions_defined_in_schema_yml,
1213
models_versions_updated_schema_yml,
1314
model_three_sql,
1415
model_three_modified_sql,
@@ -305,8 +306,8 @@ class TestVersionedModels:
305306
@pytest.fixture(scope="class")
306307
def models(self):
307308
return {
309+
"model_one_v1.sql": model_one_sql,
308310
"model_one.sql": model_one_sql,
309-
"model_one_v2.sql": model_one_sql,
310311
"model_one_downstream.sql": model_four2_sql,
311312
"schema.yml": models_versions_schema_yml,
312313
}
@@ -324,11 +325,22 @@ def test_pp_versioned_models(self, project):
324325
model_one_downstream_node = manifest.nodes["model.test.model_one_downstream"]
325326
assert model_one_downstream_node.depends_on.nodes == ["model.test.model_one.v2"]
326327

328+
# update schema.yml block - model_one is now 'defined_in: model_one_different'
329+
rm_file(project.project_root, "models", "model_one.sql")
330+
write_file(model_one_sql, project.project_root, "models", "model_one_different.sql")
331+
write_file(
332+
models_versions_defined_in_schema_yml, project.project_root, "models", "schema.yml"
333+
)
334+
results = run_dbt(["--partial-parse", "run"])
335+
assert len(results) == 3
336+
327337
# update versions schema.yml block - latest_version from 2 to 1
328338
write_file(
329339
models_versions_updated_schema_yml, project.project_root, "models", "schema.yml"
330340
)
331-
results = run_dbt(["--partial-parse", "run"])
341+
results, log_output = run_dbt_and_capture(
342+
["--partial-parse", "--log-format", "json", "run"]
343+
)
332344
assert len(results) == 3
333345

334346
manifest = get_manifest(project.project_root)
@@ -339,9 +351,11 @@ def test_pp_versioned_models(self, project):
339351
# assert unpinned ref points to latest version
340352
model_one_downstream_node = manifest.nodes["model.test.model_one_downstream"]
341353
assert model_one_downstream_node.depends_on.nodes == ["model.test.model_one.v1"]
354+
# assert unpinned ref to latest-not-max version yields an "FYI" info-level log
355+
assert "UnpinnedRefNewVersionAvailable" in log_output
342356

343357
# update versioned model
344-
write_file(model_two_sql, project.project_root, "models", "model_one_v2.sql")
358+
write_file(model_two_sql, project.project_root, "models", "model_one_different.sql")
345359
results = run_dbt(["--partial-parse", "run"])
346360
assert len(results) == 3
347361

tests/unit/test_events.py

+3
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,9 @@ def test_event_codes(self):
228228
types.JinjaLogWarning(),
229229
types.JinjaLogInfo(msg=""),
230230
types.JinjaLogDebug(msg=""),
231+
types.UnpinnedRefNewVersionAvailable(
232+
ref_node_name="", ref_node_package="", ref_node_version="", ref_max_version=""
233+
),
231234
# M - Deps generation ======================
232235
types.GitSparseCheckoutSubdirectory(subdir=""),
233236
types.GitProgressCheckoutRevision(revision=""),

0 commit comments

Comments
 (0)