Skip to content
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

make native env rendering opt-in #2618

Merged
merged 3 commits into from
Jul 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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)


Expand Down
65 changes: 54 additions & 11 deletions core/dbt/clients/jinja.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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,
Expand All @@ -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

Expand Down
6 changes: 5 additions & 1 deletion core/dbt/config/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions core/dbt/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
12 changes: 6 additions & 6 deletions test/integration/039_config_test/test_configs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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',
Expand All @@ -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
Expand All @@ -155,7 +155,7 @@ def project_config(self):
'quote_columns': False,
'test': {
'seed': {
'enabled': "{{ target.name == 'default2' }}",
'enabled': "{{ (target.name == 'default2') | as_bool }}",
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion test/integration/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
8 changes: 4 additions & 4 deletions test/unit/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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') }}",
Expand All @@ -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') }}",
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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'
Expand Down
Loading