Skip to content
Merged
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
1ac2f04
Add docstrings to `contracts/graph/metrics.py` functions to document …
QMalcolm Sep 5, 2023
b84b575
Add typing to `reverse_dag_parsing` and update function to work on 1.…
QMalcolm Sep 6, 2023
a92e5aa
Add typing to `parent_metrics` and `parent_metrics_names`
QMalcolm Sep 6, 2023
c8389f9
Add typing to `base_metric_dependency` and `derived_metric_dependency…
QMalcolm Sep 6, 2023
8f0c5c6
Simplify implementations of `basic_metric_dependency` and `derived_me…
QMalcolm Sep 6, 2023
4e062cb
Add typing to `ResolvedMetricReference` initialization
QMalcolm Sep 6, 2023
4dfe539
Add typing to `derived_metric_dependency_graph`
QMalcolm Sep 6, 2023
4a11c9f
Simplify conditional controls in `ResolvedMetricReference` functions
QMalcolm Sep 6, 2023
41c8ad0
Don't recurse on over `depends_on` for non-derived metrics in `revers…
QMalcolm Sep 6, 2023
b476f28
Simplify `parent_metrics_names` by having it call `parent_metrics`
QMalcolm Sep 6, 2023
84c5091
Unskip `TestMetricHelperFunctions.test_derived_metric` and update fix…
QMalcolm Sep 6, 2023
b073cff
Add changie doc for metric helper function updates
QMalcolm Sep 6, 2023
711ca98
Get manifest in `test_derived_metric` from the parse dbt_run invocation
QMalcolm Sep 6, 2023
70091df
Remove `Relation` a intiatlization attribute for `ResolvedMetricRefer…
QMalcolm Sep 6, 2023
da99e59
Add return typing to class `__` functions of `ResolvedMetricReference`
QMalcolm Sep 6, 2023
f80b4e1
Move from `manifest.metrics.get` to `manifest.expect` in metric helpers
QMalcolm Sep 7, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Move from manifest.metrics.get to manifest.expect in metric helpers
Previously with `manifest.metrics.get` we were just skipping when `None`
was returned. Getting `None` back was expected in that `parent_unique_id`s
that didn't belong to metrics should return `None` when calling
`manifest.metrics.get`, and these are fine to skip. However, there's
an edgecase where a `parent_unique_id` is supposed to be a metric, but
isn't found, thus returning `None`. How likely this edge case could
get hit, I'm not sure, but it's a possible edge case. Using `manifest.metrics.get`
it we can't actually tell if we're in the edge case or not. By moving
to `manifest.expect` we get the error handling built in, and the only
trade off is that we need to change our conditional to skip returned
nodes that aren't metrics.
  • Loading branch information
QMalcolm committed Sep 7, 2023
commit f80b4e1d58bc55c3873f33cb65c6d90dadf15dee
12 changes: 6 additions & 6 deletions core/dbt/contracts/graph/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ def parent_metrics(cls, metric_node: Metric, manifest: Manifest) -> Iterator[Met
yield metric_node

for parent_unique_id in metric_node.depends_on.nodes:
metric = manifest.metrics.get(parent_unique_id)
if metric is not None:
yield from cls.parent_metrics(metric, manifest)
node = manifest.expect(parent_unique_id)
if isinstance(node, Metric):
yield from cls.parent_metrics(node, manifest)

@classmethod
def parent_metrics_names(cls, metric_node: Metric, manifest: Manifest) -> Iterator[str]:
Expand All @@ -63,9 +63,9 @@ def reverse_dag_parsing(
yield {metric_node.name: metric_depth_count}

for parent_unique_id in metric_node.depends_on.nodes:
metric = manifest.metrics.get(parent_unique_id)
if metric is not None:
yield from cls.reverse_dag_parsing(metric, manifest, metric_depth_count + 1)
node = manifest.expect(parent_unique_id)
if isinstance(node, Metric):
yield from cls.reverse_dag_parsing(node, manifest, metric_depth_count + 1)

def full_metric_dependency(self):
"""Returns a unique list of all upstream metric names."""
Expand Down