feat(dbt-ingestion): add documentation link from dbt source to institutionalMemory #8686

ethan-cartwright · 2023-08-22T01:33:30Z

Description

We want the ability to add a link in the +Add Link section from the DBT source side. This PR adds dbt-side specified documentation_link as an institutionalMemory aspect on a dataset with the datahub-side meta_mapping specified description in the config (see the dbt.md file for details).

NOTE: right now it replaces the current institution memory link with the new link. If we'd like the ability to add multiple links, or to check the dataset's current institutionalMemory and add our link to it, let me know.

Example

adds:

to:

Testing

unit tests
local ingestion of dbt-cloud project linked here

Results:

Checklist

The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
Links to related issues (if applicable)
Tests for the changes have been added/updated (if applicable)
Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

hsheth2

some initial comments from a first pass

we'll need docs for this in the dbt.md file too

hsheth2 · 2023-08-22T01:46:19Z

metadata-ingestion/examples/recipes/dbt_cloud_sample_to_datahub.dhub.yaml

+        stateful_ingestion:
+            enabled: false
+        account_id: '107298'
+        job_id: '399798'


can you merge this stuff into the dbt sample recipe instead

datahub/metadata-ingestion/docs/sources/dbt/dbt-cloud_recipe.yml

Line 12 in 05a0f3e

run_id: # set to your dbt cloud run id. This is optional, and defaults to the latest run

hsheth2 · 2023-08-22T01:46:38Z

metadata-ingestion/src/datahub/utilities/mapping.py

@@ -12,6 +14,13 @@
    OwnershipTypeClass,
 )

+# Imports for metadata model classes


Suggested change

# Imports for metadata model classes

hsheth2 · 2023-08-22T01:47:12Z

metadata-ingestion/src/datahub/utilities/mapping.py

+
+        if Constants.ADD_DOC_LINK_OPERATION in operation_map:
+            try:
+                docs_dic = ast.literal_eval(


ast eval feels strange here - could we just use json.loads or similar instead?

also, the ownership one supports ownership types without ever doing any parsing - can we use the same approach of a nested object here?

good call, went with the second option. Parsing was indeed unnecessary

hsheth2 · 2023-08-22T01:47:33Z

metadata-ingestion/src/datahub/utilities/mapping.py

+
+        if Constants.ADD_DOC_LINK_OPERATION in operation_map:
+            try:
+                docs_dic = ast.literal_eval(


Suggested change

docs_dic = ast.literal_eval(

docs_dict = ast.literal_eval(

hsheth2 · 2023-08-22T01:48:59Z

metadata-ingestion/src/datahub/utilities/mapping.py

+                now = int(time.time() * 1000)  # milliseconds since epoch
+                institutional_memory_element = InstitutionalMemoryMetadataClass(
+                    url=docs_dic["link"],
+                    description=docs_dic["descr"],


descr -> description

vercel · 2023-08-23T17:24:32Z

Deployment failed with the following error:

The provided GitHub repository does not contain the requested branch or commit reference. Please ensure the repository is not empty.

hsheth2 · 2023-08-23T23:12:02Z

metadata-ingestion/docs/sources/dbt/dbt-cloud_recipe.yml

-    account_id: # set to your dbt cloud account id
-    project_id: # set to your dbt cloud project id
-    job_id: # set to your dbt cloud job id
+    account_id: '107298' # set to your dbt cloud account id


make it clear that this is not something they can just copy

Suggested change

account_id: '107298' # set to your dbt cloud account id

account_id: "${DBT_ACCOUNT_ID}" # set to your dbt cloud account id

hsheth2 · 2023-08-23T23:15:05Z

metadata-ingestion/docs/sources/dbt/dbt.md

+    match: ".*"
+    operation: "add_link"
+    config:
+      link: "https://en.wikipedia.org/wiki/Anchor_text"


let's change this example to use a pattern

in reality, we don't expect people to just add the same link to every asset. instead, the link is going to be in their meta section and we have to extract it out

hsheth2 · 2023-08-23T23:28:19Z

metadata-ingestion/src/datahub/utilities/mapping.py

+
+        if Constants.ADD_DOC_LINK_OPERATION in operation_map:
+            try:
+                docs_dict = operation_map[Constants.ADD_DOC_LINK_OPERATION].pop()


using pop() here doesn't seem right

hsheth2 · 2023-08-23T23:29:18Z

metadata-ingestion/tests/unit/test_mapping.py

+            },
+        },
+    )
+    # we require a description, so this should stay empty


if description is missing, do we issue a generic error message or a detailed one?

now we give a detailed one 👍

hsheth2 · 2023-08-23T23:30:03Z

metadata-ingestion/tests/unit/test_mapping.py

+    processor = OperationProcessor(
+        operation_defs={
+            "documentation_link": {
+                "match": ".*",


can we also test that match param works correctly?

Can you elaborate on this one for me?

something similar to the test case here

datahub/metadata-ingestion/tests/integration/dbt/test_dbt.py

Line 62 in 5b9fd97

"match": "^@(.*)",

e.g. match=(.+)(?:#.*)? and raw props =http://example.com/my-page#stuff-to-ignore should produce an aspect with link = http://example.com/my-page

vercel · 2023-10-03T21:03:44Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
docs-website	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Oct 3, 2023 9:39pm

hsheth2 · 2023-10-04T16:00:07Z

CI passed on a prior commit, so going to merge

@ethan-cartwright - if CI is green, there's usually no need to hit the "update branch" button

ethan-cartwright · 2023-10-04T16:22:49Z

CI passed on a prior commit, so going to merge

@ethan-cartwright - if CI is green, there's usually no need to hit the "update branch" button

oh okay thanks!

ethan-cartwright added 7 commits August 21, 2023 21:12

add sample dbt-cloud ingestion recipe

04da047

WIP: working solution to add link with hardcoded gms & doc description

2653dc0

remove existing Institutional Memory check

c29ef78

clean up code

e9fdb18

clean up more code

e45464d

add unit test

29236b9

clean up code

c4adbc5

github-actions bot added the ingestion PR or Issue related to the ingestion of metadata label Aug 22, 2023

ethan-cartwright changed the title ~~Cus 903 dbt meta mapping~~ feat(dbt-ingestion): add documentation link from dbt source to institutionalMemory Aug 22, 2023

vercel bot deployed to Preview August 22, 2023 01:47 View deployment

hsheth2 reviewed Aug 22, 2023

View reviewed changes

ethan-cartwright added 3 commits August 22, 2023 16:20

remove ast.evaluate and use meta mapping for link description

d88a5db

update unit tests

0c2eac9

add docs

b72db7e

ethan-cartwright requested a review from hsheth2 August 22, 2023 20:37

remove ast lib

4d71fd9

vercel bot deployed to Preview August 22, 2023 21:25 View deployment

fix linting errors

df1b6f9

Merge branch 'master' into CUS-903-dbt-meta-mapping

0050981

vercel bot deployed to Preview August 23, 2023 18:22 View deployment

hsheth2 reviewed Aug 23, 2023

View reviewed changes

ethan-cartwright and others added 2 commits September 18, 2023 07:13

Merge branch 'datahub-project:master' into CUS-903-dbt-meta-mapping

5634c70

address some PR comments

d392a22

vercel bot deployed to Preview September 18, 2023 07:42 View deployment

ethan-cartwright and others added 2 commits September 25, 2023 10:36

address PR comments

8c4d1a5

Merge branch 'master' into CUS-903-dbt-meta-mapping

73fc046

vercel bot deployed to Preview September 25, 2023 15:42 View deployment

ethan-cartwright and others added 2 commits September 25, 2023 17:59

format with black

df4d4ac

Merge branch 'master' into CUS-903-dbt-meta-mapping

2973d65

ethan-cartwright requested a review from hsheth2 September 25, 2023 22:08

vercel bot deployed to Preview September 25, 2023 22:38 View deployment

ethan-cartwright and others added 3 commits September 26, 2023 17:08

add testing for match pattern

c2f3b5e

Merge branch 'master' into CUS-903-dbt-meta-mapping

e81bbc0

fix linter issues

fd9f8ca

vercel bot deployed to Preview September 26, 2023 21:39 View deployment

hsheth2 approved these changes Sep 27, 2023

View reviewed changes

hsheth2 added the merge-pending-ci A PR that has passed review and should be merged once CI is green. label Sep 27, 2023

Merge branch 'master' into CUS-903-dbt-meta-mapping

45ce14c

vercel bot deployed to Preview September 28, 2023 17:00 View deployment

Merge branch 'master' into CUS-903-dbt-meta-mapping

c27b5d6

vercel bot deployed to Preview October 2, 2023 16:27 View deployment

Merge branch 'master' into CUS-903-dbt-meta-mapping

cecf678

vercel bot deployed to Preview October 3, 2023 21:39 View deployment

Merge branch 'master' into CUS-903-dbt-meta-mapping

b1e4a0f

vercel bot deployed to Preview October 4, 2023 05:04 View deployment

ethan-cartwright added 2 commits October 4, 2023 10:09

Merge branch 'master' into CUS-903-dbt-meta-mapping

ce4b4e9

Merge branch 'master' into CUS-903-dbt-meta-mapping

a2fb52a

vercel bot deployed to Preview October 4, 2023 14:44 View deployment

hsheth2 enabled auto-merge (squash) October 4, 2023 16:00

hsheth2 merged commit e2afd44 into datahub-project:master Oct 4, 2023
51 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(dbt-ingestion): add documentation link from dbt source to institutionalMemory #8686

feat(dbt-ingestion): add documentation link from dbt source to institutionalMemory #8686

ethan-cartwright commented Aug 22, 2023 •

edited

Loading

hsheth2 left a comment

hsheth2 Aug 22, 2023

hsheth2 Aug 22, 2023

hsheth2 Aug 22, 2023

ethan-cartwright Aug 22, 2023

hsheth2 Aug 22, 2023

hsheth2 Aug 22, 2023

vercel bot commented Aug 23, 2023

hsheth2 Aug 23, 2023

hsheth2 Aug 23, 2023

hsheth2 Aug 23, 2023

hsheth2 Aug 23, 2023

ethan-cartwright Sep 25, 2023

hsheth2 Aug 23, 2023

ethan-cartwright Sep 25, 2023

hsheth2 Sep 26, 2023

vercel bot commented Oct 3, 2023 •

edited

Loading

hsheth2 commented Oct 4, 2023

ethan-cartwright commented Oct 4, 2023

	account_id: '107298' # set to your dbt cloud account id
	account_id: "${DBT_ACCOUNT_ID}" # set to your dbt cloud account id

feat(dbt-ingestion): add documentation link from dbt source to institutionalMemory #8686

feat(dbt-ingestion): add documentation link from dbt source to institutionalMemory #8686

Conversation

ethan-cartwright commented Aug 22, 2023 • edited Loading

Description

Example

Testing

Checklist

hsheth2 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Aug 23, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vercel bot commented Oct 3, 2023 • edited Loading

hsheth2 commented Oct 4, 2023

ethan-cartwright commented Oct 4, 2023

ethan-cartwright commented Aug 22, 2023 •

edited

Loading

vercel bot commented Oct 3, 2023 •

edited

Loading