deprecate materialization overrides from imported packages #9971
Conversation
|
Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9971 +/- ##
==========================================
- Coverage 88.12% 88.12% -0.01%
==========================================
Files 180 181 +1
Lines 22578 22614 +36
==========================================
+ Hits 19897 19928 +31
- Misses 2681 2686 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
core/dbt/events/types.py
Outdated
| return "D016" | ||
|
|
||
| def message(self) -> str: | ||
| description = f"Installed package '{self.package_name}' is overriding the built-in materialization '{self.materialization_name}'. Overrides of built-in materializations from installed packages will be deprecated in future versions of dbt." |
There was a problem hiding this comment.
TODO: link to docs on workaround before merging
There was a problem hiding this comment.
Message looks good!
I figure we can link to the docs for the behavior change flag, which will go here: https://docs.getdbt.com/reference/global-configs/legacy-behaviors#source_freshness_run_project_hooks
There was a problem hiding this comment.
@MichelleArk Nice precise work! I'm satisfied with raising this warning at execution time. It means that developers will only see this when running/building the specific models, but given that this is for builtin materializations (view/table/incremental/mv), it shouldn't be searching for needle in haystack.
Confirmed with quick local testing that we don't raise this deprecation warning if:
- a built-in materailization has an override defined within the adapter (e.g. dbt-snowflake's view)
- the package-defined materialization has a different name (e.g.
myview) that doesn't collide with a built-in materialization's name
| @@ -930,7 +935,24 @@ def find_materialization_macro_by_name( | |||
| for specificity, atype in enumerate(self._get_parent_adapter_types(adapter_type)) | |||
| ) | |||
| ) | |||
| return candidates.last() | |||
| core_candidates = [ | |||
| candidate for candidate in candidates if candidate.locality == Locality.Core | |||
There was a problem hiding this comment.
core/dbt/events/types.py
Outdated
| return "D016" | ||
|
|
||
| def message(self) -> str: | ||
| description = f"Installed package '{self.package_name}' is overriding the built-in materialization '{self.materialization_name}'. Overrides of built-in materializations from installed packages will be deprecated in future versions of dbt." |
There was a problem hiding this comment.
Message looks good!
I figure we can link to the docs for the behavior change flag, which will go here: https://docs.getdbt.com/reference/global-configs/legacy-behaviors#source_freshness_run_project_hooks
MichelleArk
added
backport 1.6.latest
backport 1.7.latest
and removed
backport 1.6.latest
backport 1.7.latest
labels
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.6.latest 1.6.latest
# Navigate to the new working tree
cd .worktrees/backport-1.6.latest
# Create a new branch
git switch --create backport-9971-to-1.6.latest
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 0290cf7dd0f7c386b802116d4c312477c4af8c02
# Push it to GitHub
git push --set-upstream origin backport-9971-to-1.6.latest
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.6.latestThen, create a pull request where the |
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-1.7.latest 1.7.latest
# Navigate to the new working tree
cd .worktrees/backport-1.7.latest
# Create a new branch
git switch --create backport-9971-to-1.7.latest
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 0290cf7dd0f7c386b802116d4c312477c4af8c02
# Push it to GitHub
git push --set-upstream origin backport-9971-to-1.7.latest
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-1.7.latestThen, create a pull request where the |
resolves #9981
Problem
We are deprecating the ability for installed packages to override built-in materializations without explicit opt-in from the user.
Users should still be able to wrap the built-in materialization in their root project to avoid this deprecation warning. e.g:
{% materialization view, default %} {{ return(my_cool_package.materialization_view_default()) }} {% endmaterialization %}Solution
🎩
Checklist