From d80431a27375b99ac37bf3562374c9a3aad03ed9 Mon Sep 17 00:00:00 2001 From: Kshitij Aranke Date: Tue, 21 Jan 2025 12:39:04 +0000 Subject: [PATCH 01/10] Fix #11000 #11001: Add `doc_blocks` to manifest for nodes and columns --- core/dbt/artifacts/resources/v1/components.py | 2 + core/dbt/parser/manifest.py | 46 ++++++++++++++++++- .../functional/docs/test_good_docs_blocks.py | 46 ++++++++++++++++++- 3 files changed, 90 insertions(+), 4 deletions(-) diff --git a/core/dbt/artifacts/resources/v1/components.py b/core/dbt/artifacts/resources/v1/components.py index 8eb43f35d8e..8461ec75ea4 100644 --- a/core/dbt/artifacts/resources/v1/components.py +++ b/core/dbt/artifacts/resources/v1/components.py @@ -68,6 +68,7 @@ class ColumnInfo(AdditionalPropertiesMixin, ExtensibleDbtClassMixin): tags: List[str] = field(default_factory=list) _extra: Dict[str, Any] = field(default_factory=dict) granularity: Optional[TimeGranularity] = None + doc_blocks: List[List[str]] = field(default_factory=list) @dataclass @@ -197,6 +198,7 @@ class ParsedResource(ParsedResourceMandatory): unrendered_config_call_dict: Dict[str, Any] = field(default_factory=dict) relation_name: Optional[str] = None raw_code: str = "" + doc_blocks: List[List[str]] = field(default_factory=list) def __post_serialize__(self, dct: Dict, context: Optional[Dict] = None): dct = super().__post_serialize__(dct, context) diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index 023c5db9300..a01b5deb3bb 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -10,6 +10,7 @@ from typing import Any, Callable, Dict, List, Mapping, Optional, Set, Tuple, Type, Union import msgpack +from jinja2.nodes import Call, TemplateData import dbt.deprecations import dbt.exceptions @@ -115,6 +116,7 @@ from dbt.parser.sources import SourcePatcher from dbt.parser.unit_tests import process_models_for_unit_test from dbt.version import __version__ +from dbt_common.clients.jinja import parse from dbt_common.clients.system import make_directory, path_exists, read_json, write_file from dbt_common.constants import SECRET_ENV_PREFIX from dbt_common.dataclass_schema import StrEnum, dbtClassMixin @@ -1657,14 +1659,54 @@ def _check_manifest(manifest: Manifest, config: RuntimeConfig) -> None: DocsContextCallback = Callable[[ResultNode], Dict[str, Any]] +def _get_doc_blocks(s: str) -> Tuple[List[List[str]], bool]: + ast = parse(s) + has_doc_blocks = False + doc_blocks: List[List[str]] = [] + + if not hasattr(ast, "body"): + return doc_blocks, has_doc_blocks + + for statement in ast.body: + for node in statement.nodes: + if isinstance(node, TemplateData) and hasattr(node, "data"): + doc_blocks.append(["str", node.data]) + if ( + isinstance(node, Call) + and hasattr(node, "node") + and hasattr(node, "args") + and node.node.name == "doc" + ): + doc_block = ["doc"] + doc_block.extend([arg.value for arg in node.args]) + doc_blocks.append(doc_block) + has_doc_blocks = True + + return doc_blocks, has_doc_blocks + + +def _get_description_and_doc_blocks(description, context): + doc_blocks, has_doc_blocks = _get_doc_blocks(description) + + if has_doc_blocks: + description = get_rendered(description, context) + else: + doc_blocks = [] + + return description, doc_blocks + + # node and column descriptions def _process_docs_for_node( context: Dict[str, Any], node: ManifestNode, ): - node.description = get_rendered(node.description, context) + node.description, node.doc_blocks = _get_description_and_doc_blocks(node.description, context) + for column_name, column in node.columns.items(): - column.description = get_rendered(column.description, context) + column.description, column.doc_blocks = _get_description_and_doc_blocks( + column.description, context + ) # source and table descriptions, column descriptions diff --git a/tests/functional/docs/test_good_docs_blocks.py b/tests/functional/docs/test_good_docs_blocks.py index e1ed96c5eb7..b3602f76035 100644 --- a/tests/functional/docs/test_good_docs_blocks.py +++ b/tests/functional/docs/test_good_docs_blocks.py @@ -58,6 +58,8 @@ description: The user's first name - name: last_name description: "{{ doc('test', 'my_model_doc__last_name') }}" + - name: tricky + description: "{{ doc('my_model_doc__id') }} The user's first name {{ doc('test', 'my_model_doc__last_name') }}" """ @@ -82,6 +84,7 @@ def test_valid_doc_ref(self, project): model_data = manifest["nodes"]["model.test.model"] assert model_data["description"] == "My model is just a copy of the seed" + assert model_data["doc_blocks"] == [["doc", "my_model_doc"]] assert { "name": "id", @@ -92,6 +95,7 @@ def test_valid_doc_ref(self, project): "quote": None, "tags": [], "granularity": None, + "doc_blocks": [["doc", "my_model_doc__id"]], } == model_data["columns"]["id"] assert { @@ -103,6 +107,7 @@ def test_valid_doc_ref(self, project): "quote": None, "tags": [], "granularity": None, + "doc_blocks": [], } == model_data["columns"]["first_name"] assert { @@ -114,9 +119,26 @@ def test_valid_doc_ref(self, project): "quote": None, "tags": [], "granularity": None, + "doc_blocks": [["doc", "test", "my_model_doc__last_name"]], } == model_data["columns"]["last_name"] - assert len(model_data["columns"]) == 3 + assert { + "name": "tricky", + "description": "The user ID number The user's first name The user's last name", + "data_type": None, + "constraints": [], + "meta": {}, + "quote": None, + "tags": [], + "granularity": None, + "doc_blocks": [ + ["doc", "my_model_doc__id"], + ["str", " The user's first name "], + ["doc", "test", "my_model_doc__last_name"], + ], + } == model_data["columns"]["tricky"] + + assert len(model_data["columns"]) == 4 class TestGoodDocsBlocksAltPath: @@ -146,6 +168,7 @@ def test_alternative_docs_path(self, project): model_data = manifest["nodes"]["model.test.model"] assert model_data["description"] == "Alt text about the model" + assert model_data["doc_blocks"] == [["doc", "my_model_doc"]] assert { "name": "id", @@ -156,6 +179,7 @@ def test_alternative_docs_path(self, project): "quote": None, "tags": [], "granularity": None, + "doc_blocks": [["doc", "my_model_doc__id"]], } == model_data["columns"]["id"] assert { @@ -167,6 +191,7 @@ def test_alternative_docs_path(self, project): "quote": None, "tags": [], "granularity": None, + "doc_blocks": [], } == model_data["columns"]["first_name"] assert { @@ -178,6 +203,23 @@ def test_alternative_docs_path(self, project): "quote": None, "tags": [], "granularity": None, + "doc_blocks": [["doc", "test", "my_model_doc__last_name"]], } == model_data["columns"]["last_name"] - assert len(model_data["columns"]) == 3 + assert { + "name": "tricky", + "description": "The user ID number with alternative text The user's first name The user's last name in this other file", + "data_type": None, + "constraints": [], + "meta": {}, + "quote": None, + "tags": [], + "granularity": None, + "doc_blocks": [ + ["doc", "my_model_doc__id"], + ["str", " The user's first name "], + ["doc", "test", "my_model_doc__last_name"], + ], + } == model_data["columns"]["tricky"] + + assert len(model_data["columns"]) == 4 From c344b2c9b23f536424c91a8b7d063e60bf3f70e1 Mon Sep 17 00:00:00 2001 From: Kshitij Aranke Date: Tue, 21 Jan 2025 12:39:27 +0000 Subject: [PATCH 02/10] Fix #11000 #11001: Add `doc_blocks` to manifest for nodes and columns --- core/dbt/parser/manifest.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index a01b5deb3bb..e941bcdfd62 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -1661,8 +1661,8 @@ def _check_manifest(manifest: Manifest, config: RuntimeConfig) -> None: def _get_doc_blocks(s: str) -> Tuple[List[List[str]], bool]: ast = parse(s) - has_doc_blocks = False doc_blocks: List[List[str]] = [] + has_doc_blocks = False if not hasattr(ast, "body"): return doc_blocks, has_doc_blocks From aec7780d09e7ff9a95a6715ae6dc1c786eb6268f Mon Sep 17 00:00:00 2001 From: Kshitij Aranke Date: Tue, 21 Jan 2025 14:23:46 +0000 Subject: [PATCH 03/10] Add doc_blocks to test_contract_configs --- .../configs/test_contract_configs.py | 71 ++++++++++++++++++- 1 file changed, 69 insertions(+), 2 deletions(-) diff --git a/tests/functional/configs/test_contract_configs.py b/tests/functional/configs/test_contract_configs.py index 179b0058a8d..86649015e6a 100644 --- a/tests/functional/configs/test_contract_configs.py +++ b/tests/functional/configs/test_contract_configs.py @@ -2,6 +2,7 @@ import pytest +from dbt.artifacts.resources.v1.components import ColumnInfo from dbt.exceptions import ParsingError, ValidationError from dbt.tests.util import ( get_artifact, @@ -10,6 +11,7 @@ run_dbt_and_capture, write_file, ) +from dbt_common.contracts.constraints import ColumnLevelConstraint, ConstraintType my_model_sql = """ {{ @@ -331,9 +333,74 @@ def test__model_contract_true(self, project): assert contract_actual_config.enforced is True - expected_columns = "{'id': ColumnInfo(name='id', description='hello', meta={}, data_type='integer', constraints=[ColumnLevelConstraint(type=, name=None, expression=None, warn_unenforced=True, warn_unsupported=True, to=None, to_columns=[]), ColumnLevelConstraint(type=, name=None, expression=None, warn_unenforced=True, warn_unsupported=True, to=None, to_columns=[]), ColumnLevelConstraint(type=, name=None, expression='(id > 0)', warn_unenforced=True, warn_unsupported=True, to=None, to_columns=[])], quote=True, tags=[], _extra={}, granularity=None), 'color': ColumnInfo(name='color', description='', meta={}, data_type='string', constraints=[], quote=None, tags=[], _extra={}, granularity=None), 'date_day': ColumnInfo(name='date_day', description='', meta={}, data_type='date', constraints=[], quote=None, tags=[], _extra={}, granularity=None)}" + expected_columns = { + "id": ColumnInfo( + name="id", + description="hello", + meta={}, + data_type="integer", + doc_blocks=[], + constraints=[ + ColumnLevelConstraint( + type=ConstraintType.not_null, + name=None, + expression=None, + warn_unenforced=True, + warn_unsupported=True, + to=None, + to_columns=[], + ), + ColumnLevelConstraint( + type=ConstraintType.primary_key, + name=None, + expression=None, + warn_unenforced=True, + warn_unsupported=True, + to=None, + to_columns=[], + ), + ColumnLevelConstraint( + type=ConstraintType.check, + name=None, + expression="(id > 0)", + warn_unenforced=True, + warn_unsupported=True, + to=None, + to_columns=[], + ), + ], + quote=True, + tags=[], + _extra={}, + granularity=None, + ), + "color": ColumnInfo( + name="color", + description="", + doc_blocks=[], + meta={}, + data_type="string", + constraints=[], + quote=None, + tags=[], + _extra={}, + granularity=None, + ), + "date_day": ColumnInfo( + name="date_day", + description="", + doc_blocks=[], + meta={}, + data_type="date", + constraints=[], + quote=None, + tags=[], + _extra={}, + granularity=None, + ), + } - assert expected_columns == str(my_model_columns) + assert expected_columns == my_model_columns # compiled fields aren't in the manifest above because it only has parsed fields manifest_json = get_artifact(project.project_root, "target", "manifest.json") From 553ed4a9b22b061472e6c278a1e52701b2cf9de1 Mon Sep 17 00:00:00 2001 From: Kshitij Aranke Date: Tue, 21 Jan 2025 14:49:17 +0000 Subject: [PATCH 04/10] Fix tests and schema --- schemas/dbt/manifest/v12.json | 324 ++++++++++++++++++ .../functional/artifacts/expected_manifest.py | 23 ++ tests/unit/contracts/graph/test_manifest.py | 1 + .../unit/contracts/graph/test_nodes_parsed.py | 14 + 4 files changed, 362 insertions(+) diff --git a/schemas/dbt/manifest/v12.json b/schemas/dbt/manifest/v12.json index 9c6413db316..987a94355ab 100644 --- a/schemas/dbt/manifest/v12.json +++ b/schemas/dbt/manifest/v12.json @@ -622,6 +622,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -730,6 +739,15 @@ "type": "string", "default": "" }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, "root_path": { "anyOf": [ { @@ -1667,6 +1685,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -1775,6 +1802,15 @@ "type": "string", "default": "" }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, "language": { "type": "string", "default": "sql" @@ -2321,6 +2357,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -2429,6 +2474,15 @@ "type": "string", "default": "" }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, "language": { "type": "string", "default": "sql" @@ -3115,6 +3169,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -3223,6 +3286,15 @@ "type": "string", "default": "" }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, "language": { "type": "string", "default": "sql" @@ -3928,6 +4000,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -4036,6 +4117,15 @@ "type": "string", "default": "" }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, "language": { "type": "string", "default": "sql" @@ -5336,6 +5426,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -5444,6 +5543,15 @@ "type": "string", "default": "" }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, "language": { "type": "string", "default": "sql" @@ -5990,6 +6098,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -6098,6 +6215,15 @@ "type": "string", "default": "" }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, "language": { "type": "string", "default": "sql" @@ -6979,6 +7105,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -7087,6 +7222,15 @@ "type": "string", "default": "" }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, "language": { "type": "string", "default": "sql" @@ -8172,6 +8316,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -8546,6 +8699,12 @@ { "type": "string" }, + { + "type": "array", + "items": { + "type": "string" + } + }, { "type": "null" } @@ -9903,6 +10062,12 @@ { "type": "string" }, + { + "type": "array", + "items": { + "type": "string" + } + }, { "type": "null" } @@ -10471,6 +10636,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -10579,6 +10753,15 @@ "type": "string", "default": "" }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, "root_path": { "anyOf": [ { @@ -11516,6 +11699,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -11624,6 +11816,15 @@ "type": "string", "default": "" }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, "language": { "type": "string", "default": "sql" @@ -12170,6 +12371,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -12278,6 +12488,15 @@ "type": "string", "default": "" }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, "language": { "type": "string", "default": "sql" @@ -12964,6 +13183,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -13072,6 +13300,15 @@ "type": "string", "default": "" }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, "language": { "type": "string", "default": "sql" @@ -13777,6 +14014,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -13885,6 +14131,15 @@ "type": "string", "default": "" }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, "language": { "type": "string", "default": "sql" @@ -15185,6 +15440,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -15293,6 +15557,15 @@ "type": "string", "default": "" }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, "language": { "type": "string", "default": "sql" @@ -15839,6 +16112,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -15947,6 +16229,15 @@ "type": "string", "default": "" }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, "language": { "type": "string", "default": "sql" @@ -16828,6 +17119,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -16936,6 +17236,15 @@ "type": "string", "default": "" }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } + }, "language": { "type": "string", "default": "sql" @@ -18012,6 +18321,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": true, @@ -18184,6 +18502,12 @@ { "type": "string" }, + { + "type": "array", + "items": { + "type": "string" + } + }, { "type": "null" } diff --git a/tests/functional/artifacts/expected_manifest.py b/tests/functional/artifacts/expected_manifest.py index 02db1905443..a37d0393cf8 100644 --- a/tests/functional/artifacts/expected_manifest.py +++ b/tests/functional/artifacts/expected_manifest.py @@ -316,6 +316,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "first_name": { "name": "first_name", @@ -326,6 +327,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "email": { "name": "email", @@ -336,6 +338,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "ip_address": { "name": "ip_address", @@ -346,6 +349,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "updated_at": { "name": "updated_at", @@ -356,6 +360,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, }, "contract": {"checksum": None, "enforced": False, "alias_types": True}, @@ -373,6 +378,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "latest_version": None, "time_spine": None, "freshness": None, + "doc_blocks": [], }, "model.test.second_model": { "compiled_path": os.path.join(compiled_model_path, "second_model.sql"), @@ -416,6 +422,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "first_name": { "name": "first_name", @@ -426,6 +433,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "email": { "name": "email", @@ -436,6 +444,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "ip_address": { "name": "ip_address", @@ -446,6 +455,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "updated_at": { "name": "updated_at", @@ -456,6 +466,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, }, "contract": {"checksum": None, "enforced": False, "alias_types": True}, @@ -473,6 +484,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "latest_version": None, "time_spine": None, "freshness": None, + "doc_blocks": [], }, "seed.test.seed": { "build_path": None, @@ -506,6 +518,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "first_name": { "name": "first_name", @@ -516,6 +529,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "email": { "name": "email", @@ -526,6 +540,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "ip_address": { "name": "ip_address", @@ -536,6 +551,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "updated_at": { "name": "updated_at", @@ -546,6 +562,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, }, "docs": {"node_color": None, "show": True}, @@ -554,6 +571,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "relation_name": relation_name_node_format.format( project.database, my_schema_name, "seed" ), + "doc_blocks": [], }, "test.test.not_null_model_id.d01cc630e6": { "alias": "not_null_model_id", @@ -607,6 +625,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "checksum": {"name": "none", "checksum": ""}, "unrendered_config": unrendered_test_config, "contract": {"checksum": None, "enforced": False, "alias_types": True}, + "doc_blocks": [], }, "snapshot.test.snapshot_seed": { "alias": "snapshot_seed", @@ -653,6 +672,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "unique_id": "snapshot.test.snapshot_seed", "unrendered_config": unrendered_snapshot_config, + "doc_blocks": [], }, "test.test.test_nothing_model_.5d38568946": { "alias": "test_nothing_model_", @@ -705,6 +725,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): }, "checksum": {"name": "none", "checksum": ""}, "unrendered_config": unrendered_test_config, + "doc_blocks": [], }, "test.test.unique_model_id.67b76558ff": { "alias": "unique_model_id", @@ -758,6 +779,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): }, "checksum": {"name": "none", "checksum": ""}, "unrendered_config": unrendered_test_config, + "doc_blocks": [], }, }, "sources": { @@ -773,6 +795,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], } }, "config": { diff --git a/tests/unit/contracts/graph/test_manifest.py b/tests/unit/contracts/graph/test_manifest.py index 526f46c7884..9b6c056e802 100644 --- a/tests/unit/contracts/graph/test_manifest.py +++ b/tests/unit/contracts/graph/test_manifest.py @@ -82,6 +82,7 @@ "compiled_path", "patch_path", "docs", + "doc_blocks", "checksum", "unrendered_config", "unrendered_config_call_dict", diff --git a/tests/unit/contracts/graph/test_nodes_parsed.py b/tests/unit/contracts/graph/test_nodes_parsed.py index 75d451b9956..6aad0f09fe4 100644 --- a/tests/unit/contracts/graph/test_nodes_parsed.py +++ b/tests/unit/contracts/graph/test_nodes_parsed.py @@ -207,6 +207,7 @@ def base_parsed_model_dict(): "config_call_dict": {}, "access": AccessType.Protected.value, "constraints": [], + "doc_blocks": [], } @@ -315,6 +316,7 @@ def complex_parsed_model_dict(): "meta": {}, "tags": [], "constraints": [], + "doc_blocks": [], }, }, "checksum": { @@ -330,6 +332,7 @@ def complex_parsed_model_dict(): "config_call_dict": {}, "access": AccessType.Protected.value, "constraints": [], + "doc_blocks": [], } @@ -538,6 +541,7 @@ def basic_parsed_seed_dict(): "unrendered_config": {}, "unrendered_config_call_dict": {}, "config_call_dict": {}, + "doc_blocks": [], } @@ -632,6 +636,7 @@ def complex_parsed_seed_dict(): "meta": {}, "tags": [], "constraints": [], + "doc_blocks": [], } }, "meta": {"foo": 1000}, @@ -644,6 +649,7 @@ def complex_parsed_seed_dict(): }, "unrendered_config_call_dict": {}, "config_call_dict": {}, + "doc_blocks": [], } @@ -844,6 +850,7 @@ def base_parsed_hook_dict(): "unrendered_config": {}, "unrendered_config_call_dict": {}, "config_call_dict": {}, + "doc_blocks": [], } @@ -925,6 +932,7 @@ def complex_parsed_hook_dict(): "meta": {}, "tags": [], "constraints": [], + "doc_blocks": [], }, }, "index": 13, @@ -938,6 +946,7 @@ def complex_parsed_hook_dict(): }, "unrendered_config_call_dict": {}, "config_call_dict": {}, + "doc_blocks": [], } @@ -1083,6 +1092,7 @@ def basic_parsed_schema_test_dict(): "unrendered_config": {}, "unrendered_config_call_dict": {}, "config_call_dict": {}, + "doc_blocks": [], } @@ -1158,6 +1168,7 @@ def complex_parsed_schema_test_dict(): "meta": {}, "tags": [], "constraints": [], + "doc_blocks": [], }, }, "column_name": "id", @@ -1172,6 +1183,7 @@ def complex_parsed_schema_test_dict(): "unrendered_config": {"materialized": "table", "severity": "WARN"}, "unrendered_config_call_dict": {}, "config_call_dict": {}, + "doc_blocks": [], } @@ -1567,6 +1579,7 @@ def basic_timestamp_snapshot_dict(): }, "unrendered_config_call_dict": {}, "config_call_dict": {}, + "doc_blocks": [], } @@ -1672,6 +1685,7 @@ def basic_check_snapshot_dict(): }, "unrendered_config_call_dict": {}, "config_call_dict": {}, + "doc_blocks": [], } From 53d12181550967e9ff9b65698b08773ba9f6401a Mon Sep 17 00:00:00 2001 From: Kshitij Aranke Date: Tue, 21 Jan 2025 16:26:30 +0000 Subject: [PATCH 05/10] Fix more tests --- .../functional/artifacts/expected_manifest.py | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/functional/artifacts/expected_manifest.py b/tests/functional/artifacts/expected_manifest.py index a37d0393cf8..06478e0cc1d 100644 --- a/tests/functional/artifacts/expected_manifest.py +++ b/tests/functional/artifacts/expected_manifest.py @@ -1029,6 +1029,7 @@ def expected_references_manifest(project): "constraints": [], "time_spine": None, "freshness": None, + "doc_blocks": [], }, "model.test.ephemeral_summary": { "alias": "ephemeral_summary", @@ -1045,6 +1046,7 @@ def expected_references_manifest(project): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [["doc", "summary_first_name"]], }, "ct": { "description": "The number of instances of the first name", @@ -1055,6 +1057,7 @@ def expected_references_manifest(project): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [["doc", "summary_count"]], }, }, "config": get_rendered_model_config(materialized="table", group="test_group"), @@ -1102,6 +1105,7 @@ def expected_references_manifest(project): "constraints": [], "time_spine": None, "freshness": None, + "doc_blocks": [["doc", "ephemeral_summary"]], }, "model.test.view_summary": { "alias": "view_summary", @@ -1118,6 +1122,7 @@ def expected_references_manifest(project): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [["doc", "summary_first_name"]], }, "ct": { "description": "The number of instances of the first name", @@ -1128,6 +1133,7 @@ def expected_references_manifest(project): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [["doc", "summary_count"]], }, }, "config": get_rendered_model_config(), @@ -1171,6 +1177,7 @@ def expected_references_manifest(project): "constraints": [], "time_spine": None, "freshness": None, + "doc_blocks": [["doc", "view_summary"]], }, "seed.test.seed": { "alias": "seed", @@ -1186,6 +1193,7 @@ def expected_references_manifest(project): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "first_name": { "name": "first_name", @@ -1196,6 +1204,7 @@ def expected_references_manifest(project): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "email": { "name": "email", @@ -1206,6 +1215,7 @@ def expected_references_manifest(project): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "ip_address": { "name": "ip_address", @@ -1216,6 +1226,7 @@ def expected_references_manifest(project): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "updated_at": { "name": "updated_at", @@ -1226,6 +1237,7 @@ def expected_references_manifest(project): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, }, "config": get_rendered_seed_config(), @@ -1250,6 +1262,7 @@ def expected_references_manifest(project): "checksum": checksum_file(seed_path), "unrendered_config": get_unrendered_seed_config(), "relation_name": '"{0}"."{1}".seed'.format(project.database, my_schema_name), + "doc_blocks": [], }, "snapshot.test.snapshot_seed": { "alias": "snapshot_seed", @@ -1291,6 +1304,7 @@ def expected_references_manifest(project): "unrendered_config": get_unrendered_snapshot_config( target_schema=alternate_schema ), + "doc_blocks": [], }, }, "sources": { @@ -1305,6 +1319,7 @@ def expected_references_manifest(project): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], } }, "config": { @@ -1578,6 +1593,7 @@ def expected_versions_manifest(project): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "ct": { "description": "The number of instances of the first name", @@ -1588,6 +1604,7 @@ def expected_versions_manifest(project): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, }, "config": get_rendered_model_config( @@ -1638,6 +1655,7 @@ def expected_versions_manifest(project): "latest_version": 2, "time_spine": None, "freshness": None, + "doc_blocks": [], }, "model.test.versioned_model.v2": { "alias": "versioned_model_v2", @@ -1654,6 +1672,7 @@ def expected_versions_manifest(project): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, "extra": { "description": "", @@ -1664,6 +1683,7 @@ def expected_versions_manifest(project): "tags": [], "constraints": [], "granularity": None, + "doc_blocks": [], }, }, "config": get_rendered_model_config( @@ -1710,6 +1730,7 @@ def expected_versions_manifest(project): "latest_version": 2, "time_spine": None, "freshness": None, + "doc_blocks": [], }, "model.test.ref_versioned_model": { "alias": "ref_versioned_model", @@ -1769,6 +1790,7 @@ def expected_versions_manifest(project): "latest_version": None, "time_spine": None, "freshness": None, + "doc_blocks": [], }, "test.test.unique_versioned_model_v1_first_name.6138195dec": { "alias": "unique_versioned_model_v1_first_name", @@ -1822,6 +1844,7 @@ def expected_versions_manifest(project): }, "checksum": {"name": "none", "checksum": ""}, "unrendered_config": unrendered_test_config, + "doc_blocks": [], }, "test.test.unique_versioned_model_v1_count.0b4c0b688a": { "alias": "unique_versioned_model_v1_count", @@ -1875,6 +1898,7 @@ def expected_versions_manifest(project): }, "checksum": {"name": "none", "checksum": ""}, "unrendered_config": unrendered_test_config, + "doc_blocks": [], }, "test.test.unique_versioned_model_v2_first_name.998430d28e": { "alias": "unique_versioned_model_v2_first_name", @@ -1928,6 +1952,7 @@ def expected_versions_manifest(project): }, "checksum": {"name": "none", "checksum": ""}, "unrendered_config": unrendered_test_config, + "doc_blocks": [], }, }, "exposures": { From bba92536a87af703aa1e321a7d809ca8301bd5ea Mon Sep 17 00:00:00 2001 From: Kshitij Aranke Date: Tue, 21 Jan 2025 16:34:59 +0000 Subject: [PATCH 06/10] Fix test_nodes.py --- tests/unit/contracts/graph/test_nodes.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/contracts/graph/test_nodes.py b/tests/unit/contracts/graph/test_nodes.py index 0648fe1174d..665b4dfe573 100644 --- a/tests/unit/contracts/graph/test_nodes.py +++ b/tests/unit/contracts/graph/test_nodes.py @@ -209,6 +209,7 @@ def basic_compiled_dict(): "config_call_dict": {}, "access": "protected", "constraints": [], + "doc_blocks": [], } @@ -529,6 +530,7 @@ def basic_compiled_schema_test_dict(): }, "unrendered_config_call_dict": {}, "config_call_dict": {}, + "doc_blocks": [], } From d2cbb689ed38f4cc2682dafb2f6d8ceef9a769b8 Mon Sep 17 00:00:00 2001 From: Kshitij Aranke Date: Tue, 21 Jan 2025 21:04:48 +0000 Subject: [PATCH 07/10] Add parser for source also --- .../resources/v1/source_definition.py | 1 + core/dbt/parser/manifest.py | 18 +++++++++--------- schemas/dbt/manifest/v12.json | 18 ++++++++++++++++++ .../functional/artifacts/expected_manifest.py | 2 +- 4 files changed, 29 insertions(+), 10 deletions(-) diff --git a/core/dbt/artifacts/resources/v1/source_definition.py b/core/dbt/artifacts/resources/v1/source_definition.py index e09095fa0af..d12de028dc0 100644 --- a/core/dbt/artifacts/resources/v1/source_definition.py +++ b/core/dbt/artifacts/resources/v1/source_definition.py @@ -74,3 +74,4 @@ class SourceDefinition(ParsedSourceMandatory): created_at: float = field(default_factory=lambda: time.time()) unrendered_database: Optional[str] = None unrendered_schema: Optional[str] = None + doc_blocks: List[List[str]] = field(default_factory=list) diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index e941bcdfd62..767cc6e9267 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -1714,17 +1714,17 @@ def _process_docs_for_source( context: Dict[str, Any], source: SourceDefinition, ): - table_description = source.description - source_description = source.source_description - table_description = get_rendered(table_description, context) - source_description = get_rendered(source_description, context) - source.description = table_description - source.source_description = source_description + source.description, source.doc_blocks = _get_description_and_doc_blocks( + source.description, context + ) + source.source_description, source.doc_blocks = _get_description_and_doc_blocks( + source.source_description, context + ) for column in source.columns.values(): - column_desc = column.description - column_desc = get_rendered(column_desc, context) - column.description = column_desc + column.description, column.doc_blocks = _get_description_and_doc_blocks( + column.description, context + ) # macro argument descriptions diff --git a/schemas/dbt/manifest/v12.json b/schemas/dbt/manifest/v12.json index 987a94355ab..55a822c32e1 100644 --- a/schemas/dbt/manifest/v12.json +++ b/schemas/dbt/manifest/v12.json @@ -8426,6 +8426,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": false, @@ -18431,6 +18440,15 @@ } ], "default": null + }, + "doc_blocks": { + "type": "array", + "items": { + "type": "array", + "items": { + "type": "string" + } + } } }, "additionalProperties": false, diff --git a/tests/functional/artifacts/expected_manifest.py b/tests/functional/artifacts/expected_manifest.py index 06478e0cc1d..31fb129c379 100644 --- a/tests/functional/artifacts/expected_manifest.py +++ b/tests/functional/artifacts/expected_manifest.py @@ -1319,7 +1319,7 @@ def expected_references_manifest(project): "tags": [], "constraints": [], "granularity": None, - "doc_blocks": [], + "doc_blocks": [["doc", "column_info"]], } }, "config": { From 4f0b67d587701f6bbf0c51d1d640930b333420ff Mon Sep 17 00:00:00 2001 From: Kshitij Aranke Date: Wed, 22 Jan 2025 16:29:14 +0000 Subject: [PATCH 08/10] Simplify to list of doc block unique IDs --- core/dbt/artifacts/resources/v1/components.py | 4 +- .../resources/v1/source_definition.py | 2 +- core/dbt/parser/manifest.py | 71 ++++--- schemas/dbt/manifest/v12.json | 180 ++++-------------- .../functional/docs/test_good_docs_blocks.py | 22 +-- .../unit/contracts/graph/test_nodes_parsed.py | 2 + 6 files changed, 86 insertions(+), 195 deletions(-) diff --git a/core/dbt/artifacts/resources/v1/components.py b/core/dbt/artifacts/resources/v1/components.py index 8461ec75ea4..6f3781b06c0 100644 --- a/core/dbt/artifacts/resources/v1/components.py +++ b/core/dbt/artifacts/resources/v1/components.py @@ -68,7 +68,7 @@ class ColumnInfo(AdditionalPropertiesMixin, ExtensibleDbtClassMixin): tags: List[str] = field(default_factory=list) _extra: Dict[str, Any] = field(default_factory=dict) granularity: Optional[TimeGranularity] = None - doc_blocks: List[List[str]] = field(default_factory=list) + doc_blocks: List[str] = field(default_factory=list) @dataclass @@ -198,7 +198,7 @@ class ParsedResource(ParsedResourceMandatory): unrendered_config_call_dict: Dict[str, Any] = field(default_factory=dict) relation_name: Optional[str] = None raw_code: str = "" - doc_blocks: List[List[str]] = field(default_factory=list) + doc_blocks: List[str] = field(default_factory=list) def __post_serialize__(self, dct: Dict, context: Optional[Dict] = None): dct = super().__post_serialize__(dct, context) diff --git a/core/dbt/artifacts/resources/v1/source_definition.py b/core/dbt/artifacts/resources/v1/source_definition.py index d12de028dc0..f827d114942 100644 --- a/core/dbt/artifacts/resources/v1/source_definition.py +++ b/core/dbt/artifacts/resources/v1/source_definition.py @@ -74,4 +74,4 @@ class SourceDefinition(ParsedSourceMandatory): created_at: float = field(default_factory=lambda: time.time()) unrendered_database: Optional[str] = None unrendered_schema: Optional[str] = None - doc_blocks: List[List[str]] = field(default_factory=list) + doc_blocks: List[str] = field(default_factory=list) diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index 767cc6e9267..627ad034c2f 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -10,7 +10,7 @@ from typing import Any, Callable, Dict, List, Mapping, Optional, Set, Tuple, Type, Union import msgpack -from jinja2.nodes import Call, TemplateData +from jinja2.nodes import Call import dbt.deprecations import dbt.exceptions @@ -1242,7 +1242,7 @@ def process_docs(self, config: RuntimeConfig): self.manifest, config.project_name, ) - _process_docs_for_node(ctx, node) + _process_docs_for_node(ctx, node, self.manifest) for source in self.manifest.sources.values(): if source.created_at < self.started_at: continue @@ -1252,7 +1252,7 @@ def process_docs(self, config: RuntimeConfig): self.manifest, config.project_name, ) - _process_docs_for_source(ctx, source) + _process_docs_for_source(ctx, source, self.manifest) for macro in self.manifest.macros.values(): if macro.created_at < self.started_at: continue @@ -1659,72 +1659,71 @@ def _check_manifest(manifest: Manifest, config: RuntimeConfig) -> None: DocsContextCallback = Callable[[ResultNode], Dict[str, Any]] -def _get_doc_blocks(s: str) -> Tuple[List[List[str]], bool]: - ast = parse(s) - doc_blocks: List[List[str]] = [] - has_doc_blocks = False +def _get_doc_blocks(description: str, manifest: Manifest, node_package: str) -> List[str]: + ast = parse(description) + doc_blocks: List[str] = [] if not hasattr(ast, "body"): - return doc_blocks, has_doc_blocks + return doc_blocks for statement in ast.body: for node in statement.nodes: - if isinstance(node, TemplateData) and hasattr(node, "data"): - doc_blocks.append(["str", node.data]) if ( isinstance(node, Call) and hasattr(node, "node") and hasattr(node, "args") and node.node.name == "doc" ): - doc_block = ["doc"] - doc_block.extend([arg.value for arg in node.args]) - doc_blocks.append(doc_block) - has_doc_blocks = True + doc_args = [arg.value for arg in node.args] - return doc_blocks, has_doc_blocks + if len(doc_args) == 1: + package, name = None, doc_args[0] + elif len(doc_args) == 2: + package, name = doc_args + else: + continue + if not manifest.metadata.project_name: + continue -def _get_description_and_doc_blocks(description, context): - doc_blocks, has_doc_blocks = _get_doc_blocks(description) + resolved_doc = manifest.resolve_doc( + name, package, manifest.metadata.project_name, node_package + ) - if has_doc_blocks: - description = get_rendered(description, context) - else: - doc_blocks = [] + if resolved_doc: + doc_blocks.append(resolved_doc.unique_id) - return description, doc_blocks + return doc_blocks # node and column descriptions def _process_docs_for_node( context: Dict[str, Any], node: ManifestNode, + manifest: Manifest, ): - node.description, node.doc_blocks = _get_description_and_doc_blocks(node.description, context) + node.doc_blocks = _get_doc_blocks(node.description, manifest, node.package_name) + node.description = get_rendered(node.description, context) for column_name, column in node.columns.items(): - column.description, column.doc_blocks = _get_description_and_doc_blocks( - column.description, context - ) + column.doc_blocks = _get_doc_blocks(column.description, manifest, node.package_name) + column.description = get_rendered(column.description, context) # source and table descriptions, column descriptions def _process_docs_for_source( context: Dict[str, Any], source: SourceDefinition, + manifest: Manifest, ): - source.description, source.doc_blocks = _get_description_and_doc_blocks( - source.description, context - ) - source.source_description, source.doc_blocks = _get_description_and_doc_blocks( - source.source_description, context - ) + source.doc_blocks = _get_doc_blocks(source.description, manifest, source.package_name) + source.description = get_rendered(source.description, context) + + source.source_description = get_rendered(source.source_description, context) for column in source.columns.values(): - column.description, column.doc_blocks = _get_description_and_doc_blocks( - column.description, context - ) + column.doc_blocks = _get_doc_blocks(column.description, manifest, source.package_name) + column.description = get_rendered(column.description, context) # macro argument descriptions @@ -2082,7 +2081,7 @@ def process_node(config: RuntimeConfig, manifest: Manifest, node: ManifestNode): _process_sources_for_node(manifest, config.project_name, node) _process_refs(manifest, config.project_name, node, config.dependencies) ctx = generate_runtime_docs_context(config, node, manifest, config.project_name) - _process_docs_for_node(ctx, node) + _process_docs_for_node(ctx, node, manifest) def write_semantic_manifest(manifest: Manifest, target_path: str) -> None: diff --git a/schemas/dbt/manifest/v12.json b/schemas/dbt/manifest/v12.json index 55a822c32e1..af7f5b2bd65 100644 --- a/schemas/dbt/manifest/v12.json +++ b/schemas/dbt/manifest/v12.json @@ -626,10 +626,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -742,10 +739,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } }, "root_path": { @@ -1689,10 +1683,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -1805,10 +1796,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } }, "language": { @@ -2361,10 +2349,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -2477,10 +2462,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } }, "language": { @@ -3173,10 +3155,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -3289,10 +3268,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } }, "language": { @@ -4004,10 +3980,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -4120,10 +4093,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } }, "language": { @@ -5430,10 +5400,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -5546,10 +5513,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } }, "language": { @@ -6102,10 +6066,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -6218,10 +6179,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } }, "language": { @@ -7109,10 +7067,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -7225,10 +7180,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } }, "language": { @@ -8320,10 +8272,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -8430,10 +8379,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -10649,10 +10595,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -10765,10 +10708,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } }, "root_path": { @@ -11712,10 +11652,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -11828,10 +11765,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } }, "language": { @@ -12384,10 +12318,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -12500,10 +12431,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } }, "language": { @@ -13196,10 +13124,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -13312,10 +13237,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } }, "language": { @@ -14027,10 +13949,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -14143,10 +14062,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } }, "language": { @@ -15453,10 +15369,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -15569,10 +15482,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } }, "language": { @@ -16125,10 +16035,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -16241,10 +16148,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } }, "language": { @@ -17132,10 +17036,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -17248,10 +17149,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } }, "language": { @@ -18334,10 +18232,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, @@ -18444,10 +18339,7 @@ "doc_blocks": { "type": "array", "items": { - "type": "array", - "items": { - "type": "string" - } + "type": "string" } } }, diff --git a/tests/functional/docs/test_good_docs_blocks.py b/tests/functional/docs/test_good_docs_blocks.py index b3602f76035..60f39b6e35e 100644 --- a/tests/functional/docs/test_good_docs_blocks.py +++ b/tests/functional/docs/test_good_docs_blocks.py @@ -84,7 +84,7 @@ def test_valid_doc_ref(self, project): model_data = manifest["nodes"]["model.test.model"] assert model_data["description"] == "My model is just a copy of the seed" - assert model_data["doc_blocks"] == [["doc", "my_model_doc"]] + assert model_data["doc_blocks"] == ["doc.test.my_model_doc"] assert { "name": "id", @@ -95,7 +95,7 @@ def test_valid_doc_ref(self, project): "quote": None, "tags": [], "granularity": None, - "doc_blocks": [["doc", "my_model_doc__id"]], + "doc_blocks": ["doc.test.my_model_doc__id"], } == model_data["columns"]["id"] assert { @@ -119,7 +119,7 @@ def test_valid_doc_ref(self, project): "quote": None, "tags": [], "granularity": None, - "doc_blocks": [["doc", "test", "my_model_doc__last_name"]], + "doc_blocks": ["doc.test.my_model_doc__last_name"], } == model_data["columns"]["last_name"] assert { @@ -132,9 +132,8 @@ def test_valid_doc_ref(self, project): "tags": [], "granularity": None, "doc_blocks": [ - ["doc", "my_model_doc__id"], - ["str", " The user's first name "], - ["doc", "test", "my_model_doc__last_name"], + "doc.test.my_model_doc__id", + "doc.test.my_model_doc__last_name", ], } == model_data["columns"]["tricky"] @@ -168,7 +167,7 @@ def test_alternative_docs_path(self, project): model_data = manifest["nodes"]["model.test.model"] assert model_data["description"] == "Alt text about the model" - assert model_data["doc_blocks"] == [["doc", "my_model_doc"]] + assert model_data["doc_blocks"] == ["doc.test.my_model_doc"] assert { "name": "id", @@ -179,7 +178,7 @@ def test_alternative_docs_path(self, project): "quote": None, "tags": [], "granularity": None, - "doc_blocks": [["doc", "my_model_doc__id"]], + "doc_blocks": ["doc.test.my_model_doc__id"], } == model_data["columns"]["id"] assert { @@ -203,7 +202,7 @@ def test_alternative_docs_path(self, project): "quote": None, "tags": [], "granularity": None, - "doc_blocks": [["doc", "test", "my_model_doc__last_name"]], + "doc_blocks": ["doc.test.my_model_doc__last_name"], } == model_data["columns"]["last_name"] assert { @@ -216,9 +215,8 @@ def test_alternative_docs_path(self, project): "tags": [], "granularity": None, "doc_blocks": [ - ["doc", "my_model_doc__id"], - ["str", " The user's first name "], - ["doc", "test", "my_model_doc__last_name"], + "doc.test.my_model_doc__id", + "doc.test.my_model_doc__last_name", ], } == model_data["columns"]["tricky"] diff --git a/tests/unit/contracts/graph/test_nodes_parsed.py b/tests/unit/contracts/graph/test_nodes_parsed.py index 6aad0f09fe4..0b275fb5ac9 100644 --- a/tests/unit/contracts/graph/test_nodes_parsed.py +++ b/tests/unit/contracts/graph/test_nodes_parsed.py @@ -1895,6 +1895,7 @@ def basic_parsed_source_definition_dict(): "enabled": True, }, "unrendered_config": {}, + "doc_blocks": [], } @@ -1927,6 +1928,7 @@ def complex_parsed_source_definition_dict(): "freshness": {"warn_after": {"period": "hour", "count": 1}, "error_after": {}}, "loaded_at_field": "loaded_at", "unrendered_config": {}, + "doc_blocks": [], } From 963acc7b033f0c3b65b24f93ffccbab6a309e3a8 Mon Sep 17 00:00:00 2001 From: Kshitij Aranke Date: Wed, 22 Jan 2025 16:46:05 +0000 Subject: [PATCH 09/10] Fix expected_manifest --- tests/functional/artifacts/expected_manifest.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/functional/artifacts/expected_manifest.py b/tests/functional/artifacts/expected_manifest.py index 31fb129c379..8538375fef1 100644 --- a/tests/functional/artifacts/expected_manifest.py +++ b/tests/functional/artifacts/expected_manifest.py @@ -1046,7 +1046,7 @@ def expected_references_manifest(project): "tags": [], "constraints": [], "granularity": None, - "doc_blocks": [["doc", "summary_first_name"]], + "doc_blocks": ["doc.test.summary_first_name"], }, "ct": { "description": "The number of instances of the first name", @@ -1057,7 +1057,7 @@ def expected_references_manifest(project): "tags": [], "constraints": [], "granularity": None, - "doc_blocks": [["doc", "summary_count"]], + "doc_blocks": ["doc.test.summary_count"], }, }, "config": get_rendered_model_config(materialized="table", group="test_group"), @@ -1105,7 +1105,7 @@ def expected_references_manifest(project): "constraints": [], "time_spine": None, "freshness": None, - "doc_blocks": [["doc", "ephemeral_summary"]], + "doc_blocks": ["doc.test.ephemeral_summary"], }, "model.test.view_summary": { "alias": "view_summary", @@ -1122,7 +1122,7 @@ def expected_references_manifest(project): "tags": [], "constraints": [], "granularity": None, - "doc_blocks": [["doc", "summary_first_name"]], + "doc_blocks": ["doc.test.summary_first_name"], }, "ct": { "description": "The number of instances of the first name", @@ -1133,7 +1133,7 @@ def expected_references_manifest(project): "tags": [], "constraints": [], "granularity": None, - "doc_blocks": [["doc", "summary_count"]], + "doc_blocks": ["doc.test.summary_count"], }, }, "config": get_rendered_model_config(), @@ -1177,7 +1177,7 @@ def expected_references_manifest(project): "constraints": [], "time_spine": None, "freshness": None, - "doc_blocks": [["doc", "view_summary"]], + "doc_blocks": ["doc.test.view_summary"], }, "seed.test.seed": { "alias": "seed", @@ -1319,7 +1319,7 @@ def expected_references_manifest(project): "tags": [], "constraints": [], "granularity": None, - "doc_blocks": [["doc", "column_info"]], + "doc_blocks": ["doc.test.column_info"], } }, "config": { @@ -1363,6 +1363,7 @@ def expected_references_manifest(project): "unrendered_config": {}, "unrendered_database": None, "unrendered_schema": "{{ var('test_schema') }}", + "doc_blocks": ["doc.test.table_info"], }, }, "exposures": { From da9763f4d04b7801a2f781cc1f9bdc33d4663f39 Mon Sep 17 00:00:00 2001 From: Kshitij Aranke Date: Wed, 22 Jan 2025 17:03:38 +0000 Subject: [PATCH 10/10] Add changie --- .changes/unreleased/Features-20250122-170328.yaml | 6 ++++++ tests/functional/artifacts/expected_manifest.py | 1 + 2 files changed, 7 insertions(+) create mode 100644 .changes/unreleased/Features-20250122-170328.yaml diff --git a/.changes/unreleased/Features-20250122-170328.yaml b/.changes/unreleased/Features-20250122-170328.yaml new file mode 100644 index 00000000000..ce95d99402c --- /dev/null +++ b/.changes/unreleased/Features-20250122-170328.yaml @@ -0,0 +1,6 @@ +kind: Features +body: Add doc_blocks to manifest for nodes and columns +time: 2025-01-22T17:03:28.866522Z +custom: + Author: aranke + Issue: 11000 11001 diff --git a/tests/functional/artifacts/expected_manifest.py b/tests/functional/artifacts/expected_manifest.py index 8538375fef1..8ef9cd3c9be 100644 --- a/tests/functional/artifacts/expected_manifest.py +++ b/tests/functional/artifacts/expected_manifest.py @@ -840,6 +840,7 @@ def expected_seeded_manifest(project, model_database=None, quote_model=False): "unrendered_config": {}, "unrendered_database": None, "unrendered_schema": "{{ var('test_schema') }}", + "doc_blocks": [], }, }, "exposures": {