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

Add rule: public models should have an example SQL query in their description #30

Merged
merged 2 commits into from
May 24, 2024
+24 −4
Merged

Add rule: public models should have an example SQL query in their description #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6c3ff87
Add rule: public models should have an example SQL query in their des…
matthieucan May 24, 2024
1c744f9
Add in docstring
matthieucan May 24, 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
6 changes: 6 additions & 0 deletions src/dbt_score/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,8 @@ class Model:
database: The database name of the model.
schema: The schema name of the model.
raw_code: The raw code of the model.
language: The language of the model, e.g. sql.
access: The access level of the model, e.g. public.
alias: The alias of the model.
patch_path: The yml path of the model, e.g. `package://model_dir/dir/file.yml`.
tags: The list of tags attached to the model.
Expand All @@ -149,6 +151,8 @@ class Model:
database: str
schema: str
raw_code: str
language: str
matthieucan marked this conversation as resolved.
Show resolved Hide resolved
access: str
alias: str | None = None
patch_path: str | None = None
tags: list[str] = field(default_factory=list)
Expand Down Expand Up @@ -200,6 +204,8 @@ def from_node(
database=node_values["database"],
schema=node_values["schema"],
raw_code=node_values["raw_code"],
language=node_values["language"],
access=node_values["access"],
alias=node_values["alias"],
patch_path=node_values["patch_path"],
tags=node_values["tags"],
Expand Down
12 changes: 11 additions & 1 deletion src/dbt_score/rules/generic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"""All generic rules."""

from dbt_score import Model, RuleViolation, rule
from dbt_score import Model, RuleViolation, Severity, rule


@rule
Expand Down Expand Up @@ -39,3 +39,13 @@ def sql_has_reasonable_size(model: Model, max_lines: int = 200) -> RuleViolation
return RuleViolation(
message=f"SQL query too long: {count_lines} lines (> {max_lines})."
)


@rule(severity=Severity.LOW)
def public_model_has_example_sql(model: Model) -> RuleViolation | None:
"""The documentation of a public model should have an example query."""
if model.language == "sql" and model.access == "public":
if "```sql" not in model.description:
return RuleViolation(
"The model description does not include an example SQL query."
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about this one, what if users only document their models with a oneliner? So not with a full markdown document that is rendered in the docs? Then does the rule still make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case no - but that would be the incentive of dbt-score, to encourage such a practice?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok fair 👌 They can disable the rule if they really don't like it 😁

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, definitely. It has a low severity, so it can always be kept as a reminder to write model documentation ;)

)
10 changes: 7 additions & 3 deletions tests/resources/manifest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,16 @@
"alias": "model1_alias",
"patch_path": "/path/to/model1.yml",
"tags": [],
"depends_on": {}
"depends_on": {},
"language": "sql",
"access": "protected"
},
"model.package.model2": {
"resource_type": "model",
"unique_id": "model.package.model2",
"name": "model2",
"relation_name": "database.schema.model2",
"description": "Description2.",
"description": "A great model.\nExample use:\n```sql\nselect 1;\n```",
"original_file_path": "/path/to/model2.sql",
"config": {},
"meta": {},
Expand All @@ -57,7 +59,9 @@
"alias": "model2_alias",
"patch_path": "/path/to/model2.yml",
"tags": [],
"depends_on": {}
"depends_on": {},
"language": "sql",
"access": "public"
},
"test.package.test1": {
"resource_type": "test",
Expand Down