Skip to content

Commit

Permalink
Handle some TODOs
Browse files Browse the repository at this point in the history
remove another to_dict call
we do not in fact pass dicts in to Var()
Make things that get passed to Var() tell it about their vars
finally make empty a property
documentation resource type is now a property, not serialized
added a Mergeable helper mixin to perform simple merges
  • Loading branch information
Jacob Beck committed Jul 9, 2019
1 parent e1b91ff commit 5f1ea26
Show file tree
Hide file tree
Showing 17 changed files with 166 additions and 159 deletions.
13 changes: 6 additions & 7 deletions core/dbt/clients/jinja.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ def get_node_template(self, node):
template = get_template(
string=node.raw_sql,
ctx={},
node=node.to_dict(),
node=node,
)
self.file_cache[key] = template

Expand Down Expand Up @@ -170,7 +170,6 @@ def _is_dunder_name(name):

def create_macro_capture_env(node):

# TODO: remove the to_dict() call and change this to use objects
class ParserMacroCapture(jinja2.Undefined):
"""
This class sets up the parser to capture macros.
Expand All @@ -179,21 +178,21 @@ 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.get('package_name')
self.package_name = node.package_name
# 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.get('root_path'),
self.node.get('original_file_path'))
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.get('package_name'),
self.node.get('name'), path))
.format(self.name, self.node.package_name,
self.node.name, path))

# match jinja's message
dbt.exceptions.raise_compiler_error(
Expand Down
22 changes: 3 additions & 19 deletions core/dbt/context/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

from dbt.adapters.factory import get_adapter
from dbt.node_types import NodeType
from dbt.contracts.graph.parsed import ParsedMacro
from dbt.include.global_project import PACKAGES
from dbt.include.global_project import PROJECT_NAME as GLOBAL_PROJECT_NAME

Expand Down Expand Up @@ -225,28 +224,13 @@ def __init__(self, model, context, overrides):
# precedence over context-based var definitions
self.overrides = overrides

if isinstance(model, dict) and model.get('unique_id'):
# TODO: is this reachable anymore?
local_vars = model.get('config', {}).get('vars', {})
self.model_name = model.get('name')
elif isinstance(model, ParsedMacro):
local_vars = {}
self.model_name = model.name
elif hasattr(model, 'name'):
# handle all manner of Node objects
# TODO: maybe we should register them against an abstract base
# class so we can just do an `isinstance` check?
local_vars = model.config.vars
self.model_name = model.name
elif model is None:
if model is None:
# during config parsing we have no model and no local vars
self.model_name = '<Configuration>'
local_vars = {}
else:
raise dbt.exceptions.InternalException(
'Could not call Var() for unknown model {} with type {}'
.format(model, type(model))
)
self.model_name = model.name
local_vars = model.local_vars()

self.local_vars = dbt.utils.merge(local_vars, overrides)

Expand Down
6 changes: 1 addition & 5 deletions core/dbt/contracts/graph/compiled.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class InjectedCTE(JsonSchemaMixin, Replaceable):
sql: Optional[str] = None


class CompiledNodeMixins:
class CompiledNodeMixins(ParsedNodeMixins):
def prepend_ctes(self, prepended_ctes):
self.extra_ctes_injected = True
self.extra_ctes = prepended_ctes
Expand Down Expand Up @@ -53,15 +53,13 @@ class CompiledNode(
HasFqn,
CanRef,
HasRelationMetadata,
ParsedNodeMixins,
CompiledNodeMixins):
compiled: bool
compiled_sql: Optional[str]
extra_ctes_injected: bool
extra_ctes: List[InjectedCTE]
injected_sql: Optional[str]
alias: str
empty: bool
tags: List[str]
config: NodeConfig
docrefs: List[Docref] = field(default_factory=list)
Expand All @@ -80,7 +78,6 @@ class CompiledTestNode(
HasFqn,
CanRef,
HasRelationMetadata,
ParsedNodeMixins,
CompiledNodeMixins):
compiled: bool
compiled_sql: Optional[str]
Expand All @@ -89,7 +86,6 @@ class CompiledTestNode(
injected_sql: Optional[str]
resource_type: TestType
alias: str
empty: bool
tags: List[str]
config: TestConfig
docrefs: List[Docref] = field(default_factory=list)
Expand Down
11 changes: 7 additions & 4 deletions core/dbt/contracts/graph/parsed.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
UnparsedBaseNode, FreshnessThreshold
)
from dbt.contracts.util import Replaceable

from dbt.logger import GLOBAL_LOGGER as logger # noqa
from dbt.node_types import (
NodeType, SourceType, SnapshotType, MacroType, TestType
Expand Down Expand Up @@ -187,6 +186,9 @@ def patch(self, patch):
def get_materialization(self):
return self.config.materialized

def local_vars(self):
return self.config.vars


# TODO(jeb): hooks should get their own parsed type instead of including
# index everywhere!
Expand All @@ -199,7 +201,6 @@ class ParsedNode(
HasRelationMetadata,
ParsedNodeMixins):
alias: str
empty: bool
tags: List[str]
config: NodeConfig
docrefs: List[Docref] = field(default_factory=list)
Expand All @@ -225,7 +226,6 @@ class ParsedTestNode(
ParsedNodeMixins):
resource_type: TestType
alias: str
empty: bool
tags: List[str]
config: TestConfig
docrefs: List[Docref] = field(default_factory=list)
Expand All @@ -239,8 +239,8 @@ class ParsedTestNode(
@dataclass(init=False)
class _SnapshotConfig(NodeConfig):
unique_key: str
target_database: str = None
target_schema: str = None
target_database: str = None

def __init__(
self,
Expand Down Expand Up @@ -333,6 +333,9 @@ class ParsedMacro(UnparsedMacro):
tags: List[str]
depends_on: _MacroDependsOn

def local_vars(self):
return {}

@property
def generator(self):
"""
Expand Down
18 changes: 11 additions & 7 deletions core/dbt/contracts/graph/unparsed.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
from dbt.node_types import UnparsedNodeType, NodeType
from dbt.contracts.util import Replaceable
from dbt.node_types import UnparsedNodeType, NodeType, OperationType
from dbt.contracts.util import Replaceable
from dbt.contracts.util import Replaceable, Mergeable

from hologram import JsonSchemaMixin
from hologram.helpers import StrEnum
Expand All @@ -23,6 +21,10 @@ class UnparsedBaseNode(JsonSchemaMixin, Replaceable):
class HasSQL:
raw_sql: str

@property
def empty(self):
return not self.raw_sql.strip()


@dataclass
class UnparsedMacro(UnparsedBaseNode, HasSQL):
Expand Down Expand Up @@ -100,7 +102,7 @@ class FreshnessStatus(StrEnum):


@dataclass
class FreshnessThreshold(JsonSchemaMixin, Replaceable):
class FreshnessThreshold(JsonSchemaMixin, Mergeable):
warn_after: Optional[Time] = None
error_after: Optional[Time] = None

Expand All @@ -114,7 +116,7 @@ def status(self, age: float) -> FreshnessStatus:


@dataclass
class Quoting(JsonSchemaMixin, Replaceable):
class Quoting(JsonSchemaMixin, Mergeable):
database: Optional[bool] = None
schema: Optional[bool] = None
identifier: Optional[bool] = None
Expand Down Expand Up @@ -152,5 +154,7 @@ class UnparsedDocumentationFile(JsonSchemaMixin, Replaceable):
path: str
original_file_path: str
file_contents: str
# TODO: remove this.
resource_type: StrLiteral(NodeType.Documentation)

@property
def resource_type(self):
return NodeType.Documentation
4 changes: 2 additions & 2 deletions core/dbt/contracts/project.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from dbt.contracts.util import Replaceable
from dbt.contracts.util import Replaceable, Mergeable
from dbt.logger import GLOBAL_LOGGER as logger # noqa
from dbt import tracking
from dbt.ui import printer
Expand Down Expand Up @@ -36,7 +36,7 @@ def json_schem(self):


@dataclass
class Quoting(JsonSchemaMixin, Replaceable):
class Quoting(JsonSchemaMixin, Mergeable):
identifier: Optional[bool]
schema: Optional[bool]
database: Optional[bool]
Expand Down
16 changes: 16 additions & 0 deletions core/dbt/contracts/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,22 @@ def replace(self, **kwargs):
return dataclasses.replace(self, **kwargs)


class Mergeable(Replaceable):
def merged(self, *args):
"""Perform a shallow merge, where the last non-None write wins. This is
intended to merge dataclasses that are a collection of optional values.
"""
replacements = {}
cls = type(self)
for field in dataclasses.fields(cls):
for arg in args:
value = getattr(arg, field.name)
if value is not None:
replacements[field.name] = value

return self.replace(**replacements)


class Writable:
def write(self, path: str, omit_none: bool = True):
write_json(path, self.to_dict(omit_none=omit_none))
7 changes: 1 addition & 6 deletions core/dbt/parser/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,10 +170,6 @@ def _build_intermediate_node_dict(self, config, node_dict, node_path,
config_dict.update(config.config)
self._mangle_hooks(config_dict)

empty = (
'raw_sql' in node_dict and len(node_dict['raw_sql'].strip()) == 0
)

node_dict.update({
'refs': [],
'sources': [],
Expand All @@ -182,7 +178,6 @@ def _build_intermediate_node_dict(self, config, node_dict, node_path,
'macros': [],
},
'unique_id': node_path,
'empty': empty,
'fqn': fqn,
'tags': tags,
'config': config_dict,
Expand Down Expand Up @@ -212,7 +207,7 @@ def _render_with_context(self, parsed_node, config):
config)

dbt.clients.jinja.get_rendered(
parsed_node.raw_sql, context, parsed_node.to_dict(),
parsed_node.raw_sql, context, parsed_node,
capture_macros=True)

def _update_parsed_node_info(self, parsed_node, config):
Expand Down
2 changes: 0 additions & 2 deletions core/dbt/parser/docs.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import dbt.exceptions
from dbt.node_types import NodeType
from dbt.parser.base import BaseParser
from dbt.contracts.graph.unparsed import UnparsedDocumentationFile
from dbt.contracts.graph.parsed import ParsedDocumentation
Expand Down Expand Up @@ -35,7 +34,6 @@ def load_file(cls, package_name, root_dir, relative_dirs):

yield UnparsedDocumentationFile(
root_path=root_dir,
resource_type=NodeType.Documentation,
path=path,
original_file_path=original_file_path,
package_name=package_name,
Expand Down
10 changes: 2 additions & 8 deletions core/dbt/parser/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import dbt.contracts.project

from dbt.contracts.graph.parsed import ColumnInfo, Docref
from dbt.contracts.graph.unparsed import FreshnessThreshold, Quoting
from dbt.context.common import generate_config_context
from dbt.clients.jinja import get_rendered
from dbt.node_types import NodeType
Expand Down Expand Up @@ -455,14 +454,9 @@ def generate_source_node(self, source, table, path, package_name, root_dir,
get_rendered(source_description, context)

loaded_at_field = table.loaded_at_field or source.loaded_at_field
# TODO: maybe hologram should do these deep merges + conversions?
freshness = FreshnessThreshold.from_dict(dbt.utils.deep_merge(
source.freshness.to_dict(), table.freshness.to_dict()
))
freshness = source.freshness.merged(table.freshness)

quoting = Quoting.from_dict(dbt.utils.deep_merge(
source.quoting.to_dict(), table.quoting.to_dict()
))
quoting = source.quoting.merged(table.quoting)

default_database = self.root_project_config.credentials.database
return ParsedSourceDefinition(
Expand Down
Loading

0 comments on commit 5f1ea26

Please sign in to comment.