Skip to content

Commit

Permalink
Fix exposures depends_on refs with aliased models (#228)
Browse files Browse the repository at this point in the history
* Separate model alias from name

* Clean up pylint ignores
  • Loading branch information
gouline authored Feb 19, 2024
1 parent 09675ec commit 5990ef6
Show file tree
Hide file tree
Showing 8 changed files with 81 additions and 33 deletions.
54 changes: 31 additions & 23 deletions dbtmetabase/_exposures.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def extract_exposures(
models = self.manifest.read_models()

ctx = self.__Context(
model_refs={m.name.upper(): m.ref for m in models if m.ref},
model_refs={m.name: m.ref for m in models if m.ref},
table_names={t["id"]: t["name"] for t in self.metabase.get_tables()},
)

Expand All @@ -89,7 +89,7 @@ def extract_exposures(
uid=collection["id"],
models=("card", "dashboard"),
):
depends = []
depends = set()
native_query = ""
header = ""

Expand All @@ -102,7 +102,7 @@ def extract_exposures(
)

result = self.__extract_card_exposures(ctx, card=entity)
depends += result["depends"]
depends.update(result["depends"])
native_query = result["native_query"]

elif item["model"] == "dashboard":
Expand All @@ -118,10 +118,12 @@ def extract_exposures(
if "id" not in card:
continue

depends += self.__extract_card_exposures(
ctx,
card=self.metabase.get_card(uid=card["id"]),
)["depends"]
depends.update(
self.__extract_card_exposures(
ctx,
card=self.metabase.get_card(uid=card["id"]),
)["depends"]
)
else:
_logger.warning("Unexpected collection item '%s'", item["model"])
continue
Expand Down Expand Up @@ -162,9 +164,9 @@ def extract_exposures(
creator_email=creator_email or "",
native_query=native_query,
depends_on={
ctx.model_refs[depend.upper()]
ctx.model_refs[depend]
for depend in depends
if depend.upper() in ctx.model_refs
if depend in ctx.model_refs
},
),
}
Expand All @@ -181,7 +183,7 @@ def __extract_card_exposures(
) -> Mapping:
"""Extracts exposures from Metabase questions."""

depends = []
depends = set()
native_query = ""

query = card.get("dataset_query", {})
Expand All @@ -193,33 +195,39 @@ def __extract_card_exposures(

if str(query_source).startswith("card__"):
# Handle questions based on other questions
depends += self.__extract_card_exposures(
ctx,
card=self.metabase.get_card(uid=query_source.split("__")[-1]),
)["depends"]
depends.update(
self.__extract_card_exposures(
ctx,
card=self.metabase.get_card(uid=query_source.split("__")[-1]),
)["depends"]
)
elif query_source in ctx.table_names:
# Normal question
source_table = ctx.table_names.get(query_source)
_logger.info("Extracted model '%s' from card", source_table)
depends.append(source_table)
depends.add(source_table)

# Find models exposed through joins
for join in query.get("query", {}).get("joins", []):
join_source = join.get("source-table")

if str(join_source).startswith("card__"):
# Handle questions based on other questions
depends += self.__extract_card_exposures(
ctx,
card=self.metabase.get_card(uid=join_source.split("__")[-1]),
)["depends"]
depends.update(
self.__extract_card_exposures(
ctx,
card=self.metabase.get_card(
uid=join_source.split("__")[-1]
),
)["depends"]
)
continue

# Joined model parsed
joined_table = ctx.table_names.get(join_source)
if joined_table:
_logger.info("Extracted model '%s' from join", joined_table)
depends.append(joined_table)
depends.add(joined_table)

elif query.get("type") == "native":
# Metabase native query
Expand All @@ -228,12 +236,12 @@ def __extract_card_exposures(

# Parse common table expressions for exclusion
for matched_cte in re.findall(_CTE_PARSER, native_query):
ctes.extend(group.upper() for group in matched_cte if group)
ctes.extend(group.lower() for group in matched_cte if group)

# Parse SQL for exposures through FROM or JOIN clauses
for sql_ref in re.findall(_EXPOSURE_PARSER, native_query):
# Grab just the table / model name
parsed_model = sql_ref.split(".")[-1].strip('"').upper()
parsed_model = sql_ref.split(".")[-1].strip('"').lower()

# Scrub CTEs (qualified sql_refs can not reference CTEs)
if parsed_model in ctes and "." not in sql_ref:
Expand All @@ -245,7 +253,7 @@ def __extract_card_exposures(

if parsed_model:
_logger.info("Extracted model '%s' from native query", parsed_model)
depends.append(parsed_model)
depends.add(parsed_model)

return {
"depends": depends,
Expand Down
4 changes: 2 additions & 2 deletions dbtmetabase/_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ def export_models(
synced = True
for model in models:
schema_name = model.schema.upper()
model_name = model.name.upper()
model_name = model.alias.upper()
table_key = f"{schema_name}.{model_name}"

table = tables.get(table_key)
Expand Down Expand Up @@ -149,7 +149,7 @@ def __export_model(
success = True

schema_name = model.schema.upper()
model_name = model.name.upper()
model_name = model.alias.upper()
table_key = f"{schema_name}.{model_name}"

api_table = ctx.tables.get(table_key)
Expand Down
4 changes: 3 additions & 1 deletion dbtmetabase/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,8 @@ def _read_model(
database=database,
schema=schema,
group=group,
name=manifest_model.get(
name=manifest_model["name"],
alias=manifest_model.get(
"alias", manifest_model.get("identifier", manifest_model["name"])
),
description=manifest_model.get("description"),
Expand Down Expand Up @@ -343,6 +344,7 @@ class Model:
group: Group

name: str
alias: str
description: Optional[str] = None
display_name: Optional[str] = None
visibility_type: Optional[str] = None
Expand Down
15 changes: 12 additions & 3 deletions tests/_mocks.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import json
from pathlib import Path
from typing import Any, Dict, Mapping, Optional
from typing import Any, Dict, Mapping, Optional, Sequence

from dbtmetabase.core import DbtMetabase
from dbtmetabase.manifest import Manifest
from dbtmetabase.manifest import Manifest, Model
from dbtmetabase.metabase import Metabase

FIXTURES_PATH = Path("tests") / "fixtures"
Expand Down Expand Up @@ -40,11 +40,20 @@ def _api(
return {}


class MockManifest(Manifest):
_models: Sequence[Model] = []

def read_models(self) -> Sequence[Model]:
if not self._models:
self._models = super().read_models()
return self._models


class MockDbtMetabase(DbtMetabase):
def __init__(
self,
manifest_path: Path = FIXTURES_PATH / "manifest-v2.json",
metabase_url: str = "http://localhost:3000",
): # pylint: disable=super-init-not-called
self._manifest = Manifest(path=manifest_path)
self._manifest = MockManifest(path=manifest_path)
self._metabase = MockMetabase(url=metabase_url)
21 changes: 21 additions & 0 deletions tests/test_exposures.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,24 @@ def test_exposures_grouping_type(self):
fixtures_path / "dashboard" / f"{i}.yml",
output_path / "dashboard" / f"{i}.yml",
)

def test_exposures_aliased_ref(self):
for model in self.c.manifest.read_models():
if not model.name.startswith("stg_"):
model.alias = f"{model.name}_alias"

aliases = [m.alias for m in self.c.manifest.read_models()]
self.assertIn("orders_alias", aliases)
self.assertIn("customers_alias", aliases)

fixtures_path = FIXTURES_PATH / "exposure" / "default"
output_path = TMP_PATH / "exposure" / "aliased"
self.c.extract_exposures(
output_path=str(output_path),
output_grouping=None,
)

self._assert_exposures(
fixtures_path / "exposures.yml",
output_path / "exposures.yml",
)
10 changes: 10 additions & 0 deletions tests/test_manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ def test_v11(self):
schema="public",
group=Group.nodes,
name="customers",
alias="customers",
description="This table has basic information about a customer, as well as some derived facts based on a customer's orders",
display_name="clients",
unique_id="model.sandbox.customers",
Expand Down Expand Up @@ -68,6 +69,7 @@ def test_v11(self):
schema="public",
group=Group.nodes,
name="orders",
alias="orders",
description="This table has basic information about orders, as well as some derived facts based on payments",
points_of_interest="Basic information only",
caveats="Some facts are derived from payments",
Expand Down Expand Up @@ -119,6 +121,7 @@ def test_v11(self):
schema="public",
group=Group.nodes,
name="stg_customers",
alias="stg_customers",
description="",
unique_id="model.sandbox.stg_customers",
columns=[
Expand All @@ -133,6 +136,7 @@ def test_v11(self):
schema="public",
group=Group.nodes,
name="stg_payments",
alias="stg_payments",
description="",
unique_id="model.sandbox.stg_payments",
columns=[
Expand All @@ -151,6 +155,7 @@ def test_v11(self):
schema="public",
group=Group.nodes,
name="stg_orders",
alias="stg_orders",
description="",
unique_id="model.sandbox.stg_orders",
columns=[
Expand All @@ -177,6 +182,7 @@ def test_v2(self):
schema="public",
group=Group.nodes,
name="orders",
alias="orders",
description="This table has basic information about orders, as well as some derived facts based on payments",
unique_id="model.jaffle_shop.orders",
columns=[
Expand Down Expand Up @@ -226,6 +232,7 @@ def test_v2(self):
schema="public",
group=Group.nodes,
name="customers",
alias="customers",
description="This table has basic information about a customer, as well as some derived facts based on a customer's orders",
unique_id="model.jaffle_shop.customers",
columns=[
Expand Down Expand Up @@ -265,6 +272,7 @@ def test_v2(self):
schema="public",
group=Group.nodes,
name="stg_orders",
alias="stg_orders",
description="",
unique_id="model.jaffle_shop.stg_orders",
columns=[
Expand All @@ -283,6 +291,7 @@ def test_v2(self):
schema="public",
group=Group.nodes,
name="stg_payments",
alias="stg_payments",
description="",
unique_id="model.jaffle_shop.stg_payments",
columns=[
Expand All @@ -301,6 +310,7 @@ def test_v2(self):
schema="public",
group=Group.nodes,
name="stg_customers",
alias="stg_customers",
description="",
unique_id="model.jaffle_shop.stg_customers",
tags=[],
Expand Down
2 changes: 0 additions & 2 deletions tests/test_metabase.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import unittest

from dbtmetabase.metabase import Metabase

from ._mocks import MockMetabase


Expand Down
4 changes: 2 additions & 2 deletions tests/test_models.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
# pylint: disable=protected-access,no-member

import unittest

from ._mocks import MockDbtMetabase


class TestModels(unittest.TestCase):
def setUp(self):
# pylint: disable=protected-access
self.c = MockDbtMetabase()
self.c._ModelsMixin__SYNC_PERIOD = 1 # type: ignore

Expand All @@ -18,6 +17,7 @@ def test_export(self):
)

def test_build_lookups(self):
# pylint: disable=protected-access,no-member
expected_tables = [
"PUBLIC.CUSTOMERS",
"PUBLIC.ORDERS",
Expand Down

0 comments on commit 5990ef6

Please sign in to comment.