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

Add OwnershipAssignment to dbt VirtualView #653

Merged

Conversation

usefulalgorithm
Copy link
Contributor

@usefulalgorithm usefulalgorithm commented Oct 31, 2023

🤔 Why?

Enables assigning ownership to dbt model's virtual views, or both the virtual view and the dataset itself.

Tracked by SC20587

🤓 What?

  • Added a config field for meta_ownerships:
meta_ownerships:
  - meta_key: owner
    ownership_type: Data Steward
    assignment_target: materialized_table # assigns the owner to either "materialized_table" (the dataset), "dbt_model" (the virtual view), or "both". Default is the materialized table.
  • Added support for parsing string values in dbt model's meta field, such that a string value containing commas will be parsed as a sequence of string values

🧪 Tested?

Tested with the newly added config, the ownership information was added correctly.

Signed-off-by: Tsung-Ju Lii <andy@metaphor.io>
Copy link

This pull request has been linked to Shortcut Story #20587: Add meta ownership to dbt models.

Copy link

github-actions bot commented Oct 31, 2023

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines Covered Coverage Threshold Status
12493 11301 90% 85% 🟢

New Files

No new covered files...

Modified Files

File Coverage Status
metaphor/dbt/config.py 100% 🟢
metaphor/dbt/manifest_parser.py 93% 🟢
metaphor/dbt/util.py 95% 🟢
TOTAL 96% 🟢

updated for commit: 5dfac70 by action🐍

Copy link

codecov bot commented Oct 31, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Files Coverage Δ
metaphor/dbt/config.py 100.00% <100.00%> (ø)
metaphor/dbt/manifest_parser.py 92.88% <100.00%> (+0.05%) ⬆️
metaphor/dbt/util.py 95.19% <100.00%> (+0.45%) ⬆️

... and 31 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

Copy link
Contributor

@mars-lan mars-lan left a comment

Choose a reason for hiding this comment

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

Let's also add a config to control the desired behavior, i.e., assigning owners to dbt model, materialized tables, or both.

metaphor/dbt/util.py Outdated Show resolved Hide resolved
elic-eon
elic-eon previously approved these changes Nov 1, 2023
Copy link
Contributor

@elic-eon elic-eon left a comment

Choose a reason for hiding this comment

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

LGTM!

metaphor/dbt/README.md Outdated Show resolved Hide resolved
metaphor/dbt/config.py Show resolved Hide resolved
metaphor/dbt/util.py Outdated Show resolved Hide resolved
elic-eon
elic-eon previously approved these changes Nov 1, 2023
Copy link
Contributor

@mars-lan mars-lan left a comment

Choose a reason for hiding this comment

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

Let's address #653 (comment) before merging.

@usefulalgorithm usefulalgorithm enabled auto-merge (squash) November 1, 2023 18:07
Copy link
Contributor

@mars-lan mars-lan left a comment

Choose a reason for hiding this comment

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

Almost there!

metaphor/dbt/config.py Outdated Show resolved Hide resolved
metaphor/dbt/util.py Outdated Show resolved Hide resolved
Copy link
Contributor

@mars-lan mars-lan left a comment

Choose a reason for hiding this comment

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

Great work!

@usefulalgorithm usefulalgorithm merged commit 63375e0 into main Nov 1, 2023
6 checks passed
@usefulalgorithm usefulalgorithm deleted the tsung-julii/sc-20587/add-meta-ownership-to-dbt-models branch November 1, 2023 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants