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.

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

[Backport 1.5.latest] support doc blocks on versioned models #8785

Merged
merged 1 commit into from
Oct 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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 .changes/unreleased/Fixes-20231004-144148.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
kind: Fixes
body: Support docs blocks on versioned model column descriptions
time: 2023-10-04T14:41:48.843486-05:00
custom:
Author: emmyoop
Issue: "8540"
3 changes: 2 additions & 1 deletion core/dbt/events/base_types.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from enum import Enum
import os
import threading
from typing import Optional
from dbt.events import types_pb2
import sys
from google.protobuf.json_format import ParseDict, MessageToDict, MessageToJson
Expand Down Expand Up @@ -126,7 +127,7 @@ class EventMsg(Protocol):
data: Message


def msg_from_base_event(event: BaseEvent, level: EventLevel = None):
def msg_from_base_event(event: BaseEvent, level: Optional[EventLevel] = None):

msg_class_name = f"{type(event).__name__}Msg"
msg_cls = getattr(types_pb2, msg_class_name)
Expand Down
2 changes: 1 addition & 1 deletion core/dbt/events/eventmgr.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ def __init__(self) -> None:
self.callbacks: List[Callable[[EventMsg], None]] = []
self.invocation_id: str = str(uuid4())

def fire_event(self, e: BaseEvent, level: EventLevel = None) -> None:
def fire_event(self, e: BaseEvent, level: Optional[EventLevel] = None) -> None:
msg = msg_from_base_event(e, level=level)

if os.environ.get("DBT_TEST_BINARY_SERIALIZATION"):
Expand Down
4 changes: 2 additions & 2 deletions core/dbt/events/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ def warn_or_error(event, node=None):
# an alternative to fire_event which only creates and logs the event value
# if the condition is met. Does nothing otherwise.
def fire_event_if(
conditional: bool, lazy_e: Callable[[], BaseEvent], level: EventLevel = None
conditional: bool, lazy_e: Callable[[], BaseEvent], level: Optional[EventLevel] = None
) -> None:
if conditional:
fire_event(lazy_e(), level=level)
Expand All @@ -267,7 +267,7 @@ def fire_event_if(
# this is where all the side effects happen branched by event type
# (i.e. - mutating the event history, printing to stdout, logging
# to files, etc.)
def fire_event(e: BaseEvent, level: EventLevel = None) -> None:
def fire_event(e: BaseEvent, level: Optional[EventLevel] = None) -> None:
EVENT_MANAGER.fire_event(e, level=level)


Expand Down
2 changes: 1 addition & 1 deletion core/dbt/parser/generic_test_builders.py
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ def __init__(
target: Testable,
package_name: str,
render_ctx: Dict[str, Any],
column_name: str = None,
column_name: Optional[str] = None,
version: Optional[NodeVersion] = None,
) -> None:
test_name, test_args = self.extract_test_args(test, column_name)
Expand Down
3 changes: 2 additions & 1 deletion core/dbt/parser/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -917,10 +917,11 @@ def process_metrics(self, config: RuntimeConfig):
continue
_process_metrics_for_node(self.manifest, current_project, exposure)

# nodes: node and column descriptions
# nodes: node and column descriptions, version columns descriptions
# sources: source and table descriptions, column descriptions
# macros: macro argument descriptions
# exposures: exposure descriptions
# metrics: metric descriptions
def process_docs(self, config: RuntimeConfig):
for node in self.manifest.nodes.values():
if node.created_at < self.started_at:
Expand Down
6 changes: 6 additions & 0 deletions core/dbt/parser/schema_renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,18 @@ def _is_norender_key(self, keypath: Keypath) -> bool:
Return True if it's tests or description - those aren't rendered now
because they're rendered later in parse_generic_tests or process_docs.
"""
# top level descriptions and tests
if len(keypath) >= 1 and keypath[0] in ("tests", "description"):
return True

# columns descriptions and tests
if len(keypath) == 2 and keypath[1] in ("tests", "description"):
return True

# versions
if len(keypath) == 5 and keypath[4] == "description":
return True

if (
len(keypath) >= 3
and keypath[0] == "columns"
Expand Down
2 changes: 1 addition & 1 deletion core/dbt/parser/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ def parse_versioned_tests(self, block: VersionedTestBlock) -> None:
for test in block.target.get_tests_for_version(version.v):
self.parse_test(block, test, None, version.v)

def parse_file(self, block: FileBlock, dct: Dict = None) -> None:
def parse_file(self, block: FileBlock, dct: Optional[Dict] = None) -> None:
assert isinstance(block.file, SchemaSourceFile)

# If partially parsing, dct should be from pp_dict, otherwise
Expand Down
74 changes: 74 additions & 0 deletions tests/functional/docs/test_model_version_docs_blocks.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import pytest

from dbt.tests.util import run_dbt

model_1 = """
select 1 as id, 'joe' as first_name
"""

model_versioned = """
select 1 as id, 'joe' as first_name
"""

docs_md = """
{% docs model_description %}
unversioned model
{% enddocs %}

{% docs column_id_doc %}
column id for some thing
{% enddocs %}

{% docs versioned_model_description %}
versioned model
{% enddocs %}

"""

schema_yml = """
models:
- name: model_1
description: '{{ doc("model_description") }}'
columns:
- name: id
description: '{{ doc("column_id_doc") }}'

- name: model_versioned
description: '{{ doc("versioned_model_description") }}'
latest_version: 1
versions:
- v: 1
config:
alias: my_alias
columns:
- name: id
description: '{{ doc("column_id_doc") }}'
- name: first_name
description: 'plain text'
- v: 2
columns:
- name: other_id
"""


class TestVersionedModelDocsBlock:
@pytest.fixture(scope="class")
def models(self):
return {
"model_1.sql": model_1,
"model_versioned.sql": model_versioned,
"schema.yml": schema_yml,
"docs.md": docs_md,
}

def test_versioned_doc_ref(self, project):
manifest = run_dbt(["parse"])
model_1 = manifest.nodes["model.test.model_1"]
model_v1 = manifest.nodes["model.test.model_versioned.v1"]

assert model_1.description == "unversioned model"
assert model_v1.description == "versioned model"

assert model_1.columns["id"].description == "column id for some thing"
assert model_v1.columns["id"].description == "column id for some thing"
assert model_v1.columns["first_name"].description == "plain text"
Loading