Merge pull request #2618 from fishtown-analytics/fix/native-env-opt-in

make native env rendering opt-in

CHANGELOG.md

@@ -1,5 +1,9 @@
 ## dbt 0.17.1 (Release TBD)
 
+### Fixes
+- dbt native rendering now requires an opt-in with the `as_native` filter. Added `as_bool` and `as_number` filters, which are like `as_native` but also type-check. ([#2612](https://github.com/fishtown-analytics/dbt/issues/2612), [#2618](https://github.com/fishtown-analytics/dbt/pull/2618))
+
+
 ## dbt 0.17.1rc3 (July 01, 2020)
 
 

core/dbt/clients/jinja.py

@@ -8,7 +8,8 @@
 from contextlib import contextmanager
 from itertools import chain, islice
 from typing import (
- List, Union, Set, Optional, Dict, Any, Iterator, Type, NoReturn, Tuple
+ List, Union, Set, Optional, Dict, Any, Iterator, Type, NoReturn, Tuple,
+ Callable
 )
 
 import jinja2
@@ -28,7 +29,7 @@
 from dbt.contracts.graph.parsed import ParsedSchemaTestNode
 from dbt.exceptions import (
  InternalException, raise_compiler_error, CompilationException,
- invalid_materialization_argument, MacroReturn
+ invalid_materialization_argument, MacroReturn, JinjaRenderingException
 )
 from dbt.flags import MACRO_DEBUGGING
 from dbt.logger import GLOBAL_LOGGER as logger # noqa
@@ -111,6 +112,24 @@ class TextMarker(str):
  """
 
 
+class NativeMarker(str):
+ """A special native-env marker that indicates the field should be passed to
+ literal_eval.
+ """
+
+
+class BoolMarker(NativeMarker):
+ pass
+
+
+class NumberMarker(NativeMarker):
+ pass
+
+
+def _is_number(value) -> bool:
+ return isinstance(value, (int, float)) and not isinstance(value, bool)
+
+
 def quoted_native_concat(nodes):
  """This is almost native_concat from the NativeTemplate, except in the
  special case of a single argument that is a quoted string and returns a
@@ -119,23 +138,31 @@ def quoted_native_concat(nodes):
  head = list(islice(nodes, 2))
 
  if not head:
- return None
+ return ''
 
  if len(head) == 1:
  raw = head[0]
  if isinstance(raw, TextMarker):
  return str(raw)
+ elif not isinstance(raw, NativeMarker):
+ # return non-strings as-is
+ return raw
  else:
- raw = "".join([str(v) for v in chain(head, nodes)])
+ # multiple nodes become a string.
+ return "".join([str(v) for v in chain(head, nodes)])
 
  try:
  result = literal_eval(raw)
  except (ValueError, SyntaxError, MemoryError):
- return raw
-
- # if it was a str and it still is a str, return it as-is.
- if isinstance(result, str):
  result = raw
+ if isinstance(raw, BoolMarker) and not isinstance(result, bool):
+ raise JinjaRenderingException(
+ f"Could not convert value '{raw!s}' into type 'bool'"
+ )
+ if isinstance(raw, NumberMarker) and not _is_number(result):
+ raise JinjaRenderingException(
+ f"Could not convert value '{raw!s}' into type 'number'"
+ )
 
  return result
 
@@ -417,6 +444,22 @@ def __reduce__(self):
  return Undefined
 
 
+NATIVE_FILTERS: Dict[str, Callable[[Any], Any]] = {
+ 'as_text': TextMarker,
+ 'as_bool': BoolMarker,
+ 'as_native': NativeMarker,
+ 'as_number': NumberMarker,
+}
+
+
+TEXT_FILTERS: Dict[str, Callable[[Any], Any]] = {
+ 'as_text': lambda x: x,
+ 'as_bool': lambda x: x,
+ 'as_native': lambda x: x,
+ 'as_number': lambda x: x,
+}
+
+
 def get_environment(
  node=None,
  capture_macros: bool = False,
@@ -436,13 +479,13 @@ def get_environment(
  text_filter: Type
  if native:
  env_cls = NativeSandboxEnvironment
- text_filter = TextMarker
+ filters = NATIVE_FILTERS
  else:
  env_cls = MacroFuzzEnvironment
- text_filter = str
+ filters = TEXT_FILTERS
 
  env = env_cls(**args)
- env.filters['as_text'] = text_filter
+ env.filters.update(filters)
 
  return env
 

core/dbt/config/profile.py

@@ -8,6 +8,7 @@
 from dbt.clients.yaml_helper import load_yaml_text
 from dbt.contracts.connection import Credentials, HasCredentials
 from dbt.contracts.project import ProfileConfig, UserConfig
+from dbt.exceptions import CompilationException
 from dbt.exceptions import DbtProfileError
 from dbt.exceptions import DbtProjectError
 from dbt.exceptions import ValidationException
@@ -268,7 +269,10 @@ def render_profile(
  raw_profile, profile_name, target_name
  )
 
- profile_data = renderer.render_data(raw_profile_data)
+ try:
+ profile_data = renderer.render_data(raw_profile_data)
+ except CompilationException as exc:
+ raise DbtProfileError(str(exc)) from exc
  return target_name, profile_data
 
  @classmethod

core/dbt/exceptions.py

@@ -256,6 +256,10 @@ def __reduce__(self):
  return (JSONValidationException, (self.typename, self.errors))
 
 
+class JinjaRenderingException(CompilationException):
+ pass
+
+
 class UnknownAsyncIDException(Exception):
  CODE = 10012
  MESSAGE = 'RPC server got an unknown async ID'

test/integration/039_config_test/test_configs.py

@@ -110,9 +110,9 @@ def postgres_profile(self):
  'default2': {
  'type': 'postgres',
  # make sure you can do this and get an int out
- 'threads': "{{ 1 + 3 }}",
+ 'threads': "{{ (1 + 3) | as_number }}",
  'host': self.database_host,
- 'port': "{{ 5400 + 32 }}",
+ 'port': "{{ (5400 + 32) | as_number }}",
  'user': 'root',
  'pass': 'password',
  'dbname': 'dbt',
@@ -121,9 +121,9 @@ def postgres_profile(self):
  'disabled': {
  'type': 'postgres',
  # make sure you can do this and get an int out
- 'threads': "{{ 1 + 3 }}",
+ 'threads': "{{ (1 + 3) | as_number }}",
  'host': self.database_host,
- 'port': "{{ 5400 + 32 }}",
+ 'port': "{{ (5400 + 32) | as_number }}",
  'user': 'root',
  'pass': 'password',
  'dbname': 'dbt',
@@ -141,7 +141,7 @@ def project_config(self):
  'data-paths': ['data'],
  'models': {
  'test': {
- 'enabled': "{{ target.name == 'default2' }}",
+ 'enabled': "{{ (target.name == 'default2' | as_bool) }}",
  },
  },
  # set the `var` result in schema.yml to be 'seed', so that the
@@ -155,7 +155,7 @@ def project_config(self):
  'quote_columns': False,
  'test': {
  'seed': {
- 'enabled': "{{ target.name == 'default2' }}",
+ 'enabled': "{{ (target.name == 'default2') | as_bool }}",
  },
  },
  },

test/integration/base.py

@@ -184,7 +184,7 @@ def redshift_profile(self):
  'type': 'redshift',
  'threads': 1,
  'host': os.getenv('REDSHIFT_TEST_HOST'),
- 'port': os.getenv('REDSHIFT_TEST_PORT'),
+ 'port': int(os.getenv('REDSHIFT_TEST_PORT')),
  'user': os.getenv('REDSHIFT_TEST_USER'),
  'pass': os.getenv('REDSHIFT_TEST_PASS'),
  'dbname': os.getenv('REDSHIFT_TEST_DBNAME'),

test/unit/test_config.py

@@ -131,7 +131,7 @@ def setUp(self):
  'with-vars': {
  'type': "{{ env_var('env_value_type') }}",
  'host': "{{ env_var('env_value_host') }}",
- 'port': "{{ env_var('env_value_port') }}",
+ 'port': "{{ env_var('env_value_port') | as_number }}",
  'user': "{{ env_var('env_value_user') }}",
  'pass': "{{ env_var('env_value_pass') }}",
  'dbname': "{{ env_var('env_value_dbname') }}",
@@ -140,7 +140,7 @@ def setUp(self):
  'cli-and-env-vars': {
  'type': "{{ env_var('env_value_type') }}",
  'host': "{{ var('cli_value_host') }}",
- 'port': "{{ env_var('env_value_port') }}",
+ 'port': "{{ env_var('env_value_port') | as_number }}",
  'user': "{{ env_var('env_value_user') }}",
  'pass': "{{ env_var('env_value_pass') }}",
  'dbname': "{{ env_var('env_value_dbname') }}",
@@ -367,7 +367,7 @@ def test_invalid_env_vars(self):
  renderer,
  target_override='with-vars'
  )
- self.assertIn("not of type 'integer'", str(exc.exception))
+ self.assertIn("Could not convert value 'hello' into type 'number'", str(exc.exception))
 
 
 class TestProfileFile(BaseFileTest):
@@ -511,7 +511,7 @@ def test_invalid_env_vars(self):
  with self.assertRaises(dbt.exceptions.DbtProfileError) as exc:
  self.from_args()
 
- self.assertIn("not of type 'integer'", str(exc.exception))
+ self.assertIn("Could not convert value 'hello' into type 'number'", str(exc.exception))
 
  def test_cli_and_env_vars(self):
  self.args.target = 'cli-and-env-vars'