-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Move Exposure
data parts to dbt/artifacts
#9494
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9494 +/- ##
==========================================
+ Coverage 87.97% 88.01% +0.04%
==========================================
Files 166 167 +1
Lines 22119 22130 +11
==========================================
+ Hits 19459 19478 +19
+ Misses 2660 2652 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
cb309c4
to
b8725ca
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧼
The branch for this PR was based off of the branch qmalcolm--9387-move-semantic-model-to-dbt-artifacts (now deleted) which was the branch for the PR #9485. Because we only allow for squash merges, the commits from the branch |
There were a few places in the code base that were importing `Owner` from `unparsed` or `nodes`. The places importing from `unparsed` were working because `unparsed` itself was correctly importing from `artifacts.resources`. However in places where it was being imported from `nodes`, an exception was being raised because in the previous commit we removed the import of `Owner` in `nodes` because it was no longer needed.
b8725ca
to
5a9145a
Compare
resolves #9380
Problem
We are moving data artifacts to
dbt/artifacts
in a piecewise fashion. We needed to moveExposure
as part of that.Solution
Moved data portion of
Exposure
todbt/artifacts
(and all other classes that doing so required)Checklist