Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow column description to be overwritten #291

Merged
merged 8 commits into from
Nov 27, 2024
Merged

feat: allow column description to be overwritten #291

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
10d2a70
feat: allow column description to be overwritten
sabino Nov 19, 2024
c3a2bc9
feat: Consider Model description as well
sabino Nov 26, 2024
8c2a86a
chore: Update tests
sabino Nov 26, 2024
bbeb875
chore: Column test
sabino Nov 26, 2024
135142e
fix: fmt
sabino Nov 26, 2024
bd849dd
fix: test v2 manifest instead of v12
sabino Nov 27, 2024
3e01490
Apply suggestions from code review
gouline Nov 27, 2024
71a157d
Apply suggestions from code review
gouline Nov 27, 2024
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
30 changes: 18 additions & 12 deletions dbtmetabase/manifest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
_COMMON_META_FIELDS = [
"display_name",
"visibility_type",
"description",
]
# Must be covered by Column attributes
_COLUMN_META_FIELDS = _COMMON_META_FIELDS + [
Expand Down Expand Up @@ -110,6 +111,12 @@ def _read_model(
for column in manifest_model.get("columns", {}).values()
]

scanned_fields = self._scan_fields(
gouline marked this conversation as resolved.
Show resolved Hide resolved
manifest_model.get("meta", {}),
fields=_MODEL_META_FIELDS,
ns=_META_NS,
)

return Model(
database=database,
schema=schema,
Expand All @@ -118,16 +125,12 @@ def _read_model(
alias=manifest_model.get(
"alias", manifest_model.get("identifier", manifest_model["name"])
),
description=manifest_model.get("description"),
description=scanned_fields.get("description", manifest_model.get("description")),
columns=columns,
unique_id=unique_id,
source=source,
tags=manifest_model.get("tags", []),
**self._scan_fields(
manifest_model.get("meta", {}),
fields=_MODEL_META_FIELDS,
ns=_META_NS,
),
**{key: value for key, value in scanned_fields.items() if key != "description"},
)

def _read_column(
Expand All @@ -136,14 +139,17 @@ def _read_column(
schema: str,
relationship: Optional[Mapping],
) -> Column:
column = Column(
name=manifest_column.get("name", ""),
description=manifest_column.get("description"),
**self._scan_fields(
manifest_column.get("meta", {}),

scanned_fields = self._scan_fields(
gouline marked this conversation as resolved.
Show resolved Hide resolved
manifest_column.get("meta", {}),
fields=_COLUMN_META_FIELDS,
ns=_META_NS,
),
)

column = Column(
name=manifest_column.get("name", ""),
description=scanned_fields.get("description", manifest_column.get("description")),
**{key: value for key, value in scanned_fields.items() if key != "description"},
)

self._set_column_relationship(
Expand Down
19 changes: 13 additions & 6 deletions tests/fixtures/manifest-v12.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@
"schema": null,
"database": null,
"tags": [],
"meta": {},
"meta": {
sabino marked this conversation as resolved.
Show resolved Hide resolved
"metabase.description": "This table has basic information about payments"
},
"group": null,
"materialized": "table",
"incremental_strategy": null,
Expand All @@ -62,7 +64,6 @@
"access": "protected"
},
"tags": [],
"description": "This table has basic information about payments",
"columns": {
"payment_id": {
"name": "payment_id",
Expand All @@ -86,7 +87,9 @@
"payment_method": {
"name": "payment_method",
"description": "",
"meta": {},
"meta": {
"metabase.description": "The method used to complete a payment."
},
"data_type": null,
"constraints": [],
"quote": null,
Expand Down Expand Up @@ -121,7 +124,9 @@
"tags": []
}
},
"meta": {},
"meta": {
"metabase.description": "This table has basic information about payments"
},
"group": null,
"docs": {
"show": true,
Expand Down Expand Up @@ -193,7 +198,8 @@
"database": null,
"tags": [],
"meta": {
"metabase.display_name": "clients"
"metabase.display_name": "clients",
"metabase.description": "Contains customer details and derived order facts"
},
"group": null,
"materialized": "table",
Expand Down Expand Up @@ -290,7 +296,8 @@
}
},
"meta": {
"metabase.display_name": "clients"
"metabase.display_name": "clients",
"metabase.description": "Contains customer details and derived order facts"
},
"group": null,
"docs": {
Expand Down
4 changes: 2 additions & 2 deletions tests/test_manifest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ def test_v12():
),
Column(
name="payment_method",
description="",
description="The method used to complete a payment.",
),
Column(
name="order_id",
Expand All @@ -60,7 +60,7 @@ def test_v12():
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",
description="Contains customer details and derived order facts",
display_name="clients",
unique_id="model.sandbox.customers",
columns=[
Expand Down
Loading