Skip to content

Commit

Permalink
Merge pull request #2184 from fishtown-analytics/fix/ugly-errors-unde…
Browse files Browse the repository at this point in the history
…fined-values

For undefined variables, explicitly raise a useful error on pickling
  • Loading branch information
beckjake authored Mar 6, 2020
2 parents a259f15 + 16692e2 commit b77b3a4
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 27 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
## dbt next (release TBD)

### Fixes
- When a jinja value is undefined, give a helpful error instead of failing with cryptic "cannot pickle ParserMacroCapture" errors ([#2110](https://github.com/fishtown-analytics/dbt/issues/2110), [#2184](https://github.com/fishtown-analytics/dbt/pull/2184))

## dbt 0.16.0rc2 (March 4, 2020)

### Under the hood
Expand Down
36 changes: 9 additions & 27 deletions core/dbt/clients/jinja.py
Original file line number Diff line number Diff line change
Expand Up @@ -311,38 +311,18 @@ def _is_dunder_name(name):
return name.startswith('__') and name.endswith('__')


def create_macro_capture_env(node):

class ParserMacroCapture(jinja2.Undefined):
"""
This class sets up the parser to capture macros.
"""
def create_undefined(node=None):
class Undefined(jinja2.Undefined):
def __init__(self, hint=None, obj=None, name=None, exc=None):
super().__init__(hint=hint, name=name)
self.node = node
self.name = name
self.package_name = node.package_name
self.hint = hint
# jinja uses these for safety, so we have to override them.
# see https://github.com/pallets/jinja/blob/master/jinja2/sandbox.py#L332-L339 # noqa
self.unsafe_callable = False
self.alters_data = False

def __deepcopy__(self, memo):
path = os.path.join(self.node.root_path,
self.node.original_file_path)

logger.debug(
'dbt encountered an undefined variable, "{}" in node {}.{} '
'(source path: {})'
.format(self.name, self.node.package_name,
self.node.name, path))

# match jinja's message
raise_compiler_error(
"{!r} is undefined".format(self.name),
node=self.node
)

def __getitem__(self, name):
# Propagate the undefined value if a caller accesses this as if it
# were a dictionary
Expand All @@ -355,15 +335,17 @@ def __getattr__(self, name):
.format(type(self).__name__, name)
)

self.package_name = self.name
self.name = name

return self
return self.__class__(hint=self.hint, name=self.name)

def __call__(self, *args, **kwargs):
return self

return ParserMacroCapture
def __reduce__(self):
raise_compiler_error(f'{self.name} is undefined', node=node)

return Undefined


def get_environment(node=None, capture_macros=False):
Expand All @@ -372,7 +354,7 @@ def get_environment(node=None, capture_macros=False):
}

if capture_macros:
args['undefined'] = create_macro_capture_env(node)
args['undefined'] = create_undefined(node)

args['extensions'].append(MaterializationExtension)
args['extensions'].append(DocumentationExtension)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
-- should be ref('model')
select * from {{ ref(model) }}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
select 1 as id
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import os

from dbt.exceptions import CompilationException

from test.integration.base import DBTIntegrationTest, use_profile


class TestSimpleReference(DBTIntegrationTest):
@property
def schema(self):
Expand Down Expand Up @@ -184,3 +189,20 @@ def test__snowflake__simple_reference_with_models_and_children(self):

self.assertTrue('EPHEMERAL_SUMMARY' in created_models)
self.assertEqual(created_models['EPHEMERAL_SUMMARY'], 'table')


class TestErrorReference(DBTIntegrationTest):
@property
def schema(self):
return "simple_reference_003"

@property
def models(self):
return "invalid-models"

@use_profile('postgres')
def test_postgres_undefined_value(self):
with self.assertRaises(CompilationException) as exc:
self.run_dbt(['compile'])
path = os.path.join('invalid-models', 'descendant.sql')
self.assertIn(path, str(exc.exception))

0 comments on commit b77b3a4

Please sign in to comment.