Skip to content

Commit

Permalink
removed deprication for materialization-return and replaced it with a…
Browse files Browse the repository at this point in the history
…n exception
  • Loading branch information
emmyoop committed Sep 16, 2021
1 parent 4845174 commit 382c52a
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 48 deletions.
17 changes: 0 additions & 17 deletions core/dbt/deprecations.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,22 +57,6 @@ class DispatchPackagesDeprecation(DBTDeprecation):
'''


class MaterializationReturnDeprecation(DBTDeprecation):
_name = 'materialization-return'

_description = '''\
The materialization ("{materialization}") did not explicitly return a list
of relations to add to the cache. By default the target relation will be
added, but this behavior will be removed in a future version of dbt.
For more information, see:
https://docs.getdbt.com/v0.15/docs/creating-new-materializations#section-6-returning-relations
'''


class NotADictionaryDeprecation(DBTDeprecation):
_name = 'not-a-dictionary'

Expand Down Expand Up @@ -178,7 +162,6 @@ def warn(name, *args, **kwargs):

deprecations_list: List[DBTDeprecation] = [
DispatchPackagesDeprecation(),
MaterializationReturnDeprecation(),
NotADictionaryDeprecation(),
ColumnQuotingDeprecation(),
ModelsKeyNonModelDeprecation(),
Expand Down
12 changes: 6 additions & 6 deletions core/dbt/task/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
get_counts,
)

from dbt import deprecations
from dbt import tracking
from dbt import utils
from dbt.adapters.base import BaseRelation
Expand Down Expand Up @@ -209,11 +208,12 @@ def _materialization_relations(
self, result: Any, model
) -> List[BaseRelation]:
if isinstance(result, str):
deprecations.warn('materialization-return',
materialization=model.get_materialization())
return [
self.adapter.Relation.create_from(self.config, model)
]
msg = (
'The materialization ("{}") did not explicitly return a '
'list of relations to add to the cache.'
.format(str(model.get_materialization()))
)
raise CompilationException(msg, node=model)

if isinstance(result, dict):
return _validate_materialization_relations_dict(result, model)
Expand Down
25 changes: 0 additions & 25 deletions test/integration/012_deprecation_tests/test_deprecations.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,31 +35,6 @@ def test_postgres_deprecations(self):
self.assertEqual(expected, deprecations.active_deprecations)


class TestMaterializationReturnDeprecation(BaseTestDeprecations):
@property
def models(self):
return self.dir('custom-models')

@property
def project_config(self):
return {
'config-version': 2,
'macro-paths': [self.dir('custom-materialization-macros')],
}

@use_profile('postgres')
def test_postgres_deprecations_fail(self):
# this should fail at runtime
self.run_dbt(strict=True, expect_pass=False)

@use_profile('postgres')
def test_postgres_deprecations(self):
self.assertEqual(deprecations.active_deprecations, set())
self.run_dbt(strict=False)
expected = {'materialization-return'}
self.assertEqual(expected, deprecations.active_deprecations)


class TestAdapterMacroDeprecation(BaseTestDeprecations):
@property
def models(self):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
name: view_adapter_override
version: 2
macro-paths: ['macros']
config-version: 2
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
{# copy+pasting the default view impl #}
{% materialization view, default %}

{%- set identifier = model['alias'] -%}
{%- set tmp_identifier = model['name'] + '__dbt_tmp' -%}
{%- set backup_identifier = model['name'] + '__dbt_backup' -%}

{%- set old_relation = adapter.get_relation(database=database, schema=schema, identifier=identifier) -%}
{%- set target_relation = api.Relation.create(identifier=identifier, schema=schema, database=database,
type='view') -%}
{%- set intermediate_relation = api.Relation.create(identifier=tmp_identifier,
schema=schema, database=database, type='view') -%}

/*
This relation (probably) doesn't exist yet. If it does exist, it's a leftover from
a previous run, and we're going to try to drop it immediately. At the end of this
materialization, we're going to rename the "old_relation" to this identifier,
and then we're going to drop it. In order to make sure we run the correct one of:
- drop view ...
- drop table ...
We need to set the type of this relation to be the type of the old_relation, if it exists,
or else "view" as a sane default if it does not. Note that if the old_relation does not
exist, then there is nothing to move out of the way and subsequentally drop. In that case,
this relation will be effectively unused.
*/
{%- set backup_relation_type = 'view' if old_relation is none else old_relation.type -%}
{%- set backup_relation = api.Relation.create(identifier=backup_identifier,
schema=schema, database=database,
type=backup_relation_type) -%}

{%- set exists_as_view = (old_relation is not none and old_relation.is_view) -%}

{{ run_hooks(pre_hooks, inside_transaction=False) }}

-- drop the temp relations if they exists for some reason
{{ adapter.drop_relation(intermediate_relation) }}
{{ adapter.drop_relation(backup_relation) }}

-- `BEGIN` happens here:
{{ run_hooks(pre_hooks, inside_transaction=True) }}

-- build model
{% call statement('main') -%}
{{ create_view_as(intermediate_relation, sql) }}
{%- endcall %}

-- cleanup
-- move the existing view out of the way
{% if old_relation is not none %}
{{ adapter.rename_relation(target_relation, backup_relation) }}
{% endif %}
{{ adapter.rename_relation(intermediate_relation, target_relation) }}

{{ run_hooks(post_hooks, inside_transaction=True) }}

{{ adapter.commit() }}

{{ drop_relation_if_exists(backup_relation) }}

{{ run_hooks(post_hooks, inside_transaction=False) }}

{# do not return anything! #}
{# {{ return({'relations': [target_relation]}) }} #}

{%- endmaterialization -%}
Original file line number Diff line number Diff line change
Expand Up @@ -98,3 +98,19 @@ def test_postgres_default_dependency(self):
self.run_dbt(['deps'])
# this should error because the override is buggy
self.run_dbt(['run'], expect_pass=False)


class TestOverrideDefaultReturn(BaseTestCustomMaterialization):
@property
def project_config(self):
return {
'config-version': 2,
'macro-paths': ['override-view-return-no-relation']
}

@use_profile('postgres')
def test_postgres_default_dependency(self):
self.run_dbt(['deps'])
self.run_dbt(['run'], expect_pass=False)


0 comments on commit 382c52a

Please sign in to comment.