Skip to content

Commit

Permalink
Fix some issues around global-scoped vars
Browse files Browse the repository at this point in the history
Fix global-level var rewriting from v2 -> v1
Add top-level vars to more lookup fields
When a user calls var() in a `generate_*_name` macro, on failure it now raises (like at runtime)
When a node has no FQN, pretend the FQN is just the package name when returning vars
Add tests
  • Loading branch information
Jacob Beck committed May 20, 2020
1 parent a8f8708 commit e2cd45b
Show file tree
Hide file tree
Showing 18 changed files with 350 additions and 46 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- dbt now logs using the adapter plugin's ideas about how relations should be displayed ([dbt-spark/#74](https://github.com/fishtown-analytics/dbt-spark/issues/74), [#2450](https://github.com/fishtown-analytics/dbt/pull/2450))
- The create_adapter_plugin.py script creates a version 2 dbt_project.yml file ([#2451](https://github.com/fishtown-analytics/dbt/issues/2451), [#2455](https://github.com/fishtown-analytics/dbt/pull/2455))
- Fixed dbt crashing with an AttributeError on duplicate sources ([#2463](https://github.com/fishtown-analytics/dbt/issues/2463), [#2464](https://github.com/fishtown-analytics/dbt/pull/2464))
- Fixed a number of issues with globally-scoped vars ([#2473](https://github.com/fishtown-analytics/dbt/issues/2473), [#2472](https://github.com/fishtown-analytics/dbt/issues/2472), [#2469](https://github.com/fishtown-analytics/dbt/issues/2469), [#2477](https://github.com/fishtown-analytics/dbt/pull/2477))
- Fixed DBT Docker entrypoint ([#2470](https://github.com/fishtown-analytics/dbt/issues/2470), [#2475](https://github.com/fishtown-analytics/dbt/pull/2475))

Contributors:
Expand Down
55 changes: 35 additions & 20 deletions core/dbt/config/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
from dataclasses import dataclass, field
from itertools import chain
from typing import (
List, Dict, Any, Optional, TypeVar, Union, Tuple, Callable, Mapping
List, Dict, Any, Optional, TypeVar, Union, Tuple, Callable, Mapping,
Iterable, Set
)
from typing_extensions import Protocol

Expand Down Expand Up @@ -274,10 +275,8 @@ def vars_for(
self, node: IsFQNResource, adapter_type: str
) -> Mapping[str, Any]:
# in v2, vars are only either project or globally scoped

merged = MultiDict([self.vars])
if node.package_name in self.vars:
merged.add(self.vars.get(node.package_name, {}))
merged.add(self.vars.get(node.package_name, {}))
return merged

def to_dict(self):
Expand Down Expand Up @@ -634,7 +633,7 @@ def validate_version(self):
)
raise DbtProjectError(msg)

def as_v1(self):
def as_v1(self, all_projects: Iterable[str]):
if self.config_version == 1:
return self

Expand All @@ -647,21 +646,7 @@ def as_v1(self):
common_config_keys = ['models', 'seeds', 'snapshots']

if 'vars' in dct and isinstance(dct['vars'], dict):
# stuff any 'vars' entries into the old-style
# models/seeds/snapshots dicts
for project_name, items in dct['vars'].items():
if not isinstance(items, dict):
# can't translate top-level vars
continue
for cfgkey in ['models', 'seeds', 'snapshots']:
if project_name not in mutated[cfgkey]:
mutated[cfgkey][project_name] = {}
project_type_cfg = mutated[cfgkey][project_name]
if 'vars' not in project_type_cfg:
project_type_cfg['vars'] = {}
mutated[cfgkey][project_name]['vars'].update(items)
# remove this from the v1 form
mutated.pop('vars')
v2_vars_to_v1(mutated, dct['vars'], set(all_projects))
# ok, now we want to look through all the existing cfgkeys and mirror
# it, except expand the '+' prefix.
for cfgkey in common_config_keys:
Expand All @@ -675,6 +660,36 @@ def as_v1(self):
return project


def v2_vars_to_v1(
dst: Dict[str, Any], src_vars: Dict[str, Any], project_names: Set[str]
) -> None:
# stuff any 'vars' entries into the old-style
# models/seeds/snapshots dicts
common_config_keys = ['models', 'seeds', 'snapshots']
for project_name in project_names:
for cfgkey in common_config_keys:
if cfgkey not in dst:
dst[cfgkey] = {}
if project_name not in dst[cfgkey]:
dst[cfgkey][project_name] = {}
project_type_cfg = dst[cfgkey][project_name]

if 'vars' not in project_type_cfg:
project_type_cfg['vars'] = {}
project_type_vars = project_type_cfg['vars']

project_type_vars.update({
k: v for k, v in src_vars.items()
if not isinstance(v, dict)
})

items = src_vars.get(project_name, None)
if isinstance(items, dict):
project_type_vars.update(items)
# remove this from the v1 form
dst.pop('vars')


def _flatten_config(dct: Dict[str, Any]):
result = {}
for key, value in dct.items():
Expand Down
4 changes: 2 additions & 2 deletions core/dbt/config/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -360,12 +360,12 @@ def _get_project_directories(self) -> Iterator[Path]:
if path.is_dir() and not path.name.startswith('__'):
yield path

def as_v1(self):
def as_v1(self, all_projects: Iterable[str]):
if self.config_version == 1:
return self

return self.from_parts(
project=Project.as_v1(self),
project=Project.as_v1(self, all_projects),
profile=self,
args=self.args,
dependencies=self.dependencies,
Expand Down
26 changes: 17 additions & 9 deletions core/dbt/context/configured.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
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
from dbt.node_types import NodeType
from dbt.utils import MultiDict

from dbt.context.base import contextproperty, Var
from dbt.context.target import TargetContext
Expand All @@ -25,6 +27,13 @@ def project_name(self) -> str:
return self.config.project_name


class FQNLookup:
def __init__(self, package_name: str):
self.package_name = package_name
self.fqn = [package_name]
self.resource_type = NodeType.Model


class ConfiguredVar(Var):
def __init__(
self,
Expand All @@ -44,17 +53,16 @@ def __call__(self, var_name, default=Var._VAR_NOTSET):
return self.config.cli_vars[var_name]

if self.config.config_version == 2 and my_config.config_version == 2:

active_vars = self.config.vars.to_dict()
active_vars = active_vars.get(self.project_name, {})
if var_name in active_vars:
return active_vars[var_name]
adapter_type = self.config.credentials.type
lookup = FQNLookup(self.project_name)
active_vars = self.config.vars.vars_for(lookup, adapter_type)
all_vars = MultiDict([active_vars])

if self.config.project_name != my_config.project_name:
config_vars = my_config.vars.to_dict()
config_vars = config_vars.get(self.project_name, {})
if var_name in config_vars:
return config_vars[var_name]
all_vars.add(my_config.vars.vars_for(lookup, adapter_type))

if var_name in all_vars:
return all_vars[var_name]

if default is not Var._VAR_NOTSET:
return default
Expand Down
34 changes: 25 additions & 9 deletions core/dbt/context/providers.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,14 @@
from dbt.context.base import (
contextmember, contextproperty, Var
)
from dbt.context.configured import ManifestContext, MacroNamespace
from dbt.context.configured import ManifestContext, MacroNamespace, FQNLookup
from dbt.context.context_config import ContextConfigType
from dbt.contracts.graph.manifest import Manifest, Disabled
from dbt.contracts.graph.compiled import (
NonSourceNode, CompiledSeedNode, CompiledResource, CompiledNode
NonSourceNode, CompiledSeedNode, CompiledResource
)
from dbt.contracts.graph.parsed import (
ParsedMacro, ParsedSourceDefinition, ParsedSeedNode, ParsedNode
ParsedMacro, ParsedSourceDefinition, ParsedSeedNode
)
from dbt.exceptions import (
InternalException,
Expand All @@ -36,6 +36,7 @@
source_target_not_found,
wrapped_exports,
)
from dbt.legacy_config_updater import IsFQNResource
from dbt.logger import GLOBAL_LOGGER as logger # noqa
from dbt.node_types import NodeType

Expand Down Expand Up @@ -450,17 +451,17 @@ def packages_for_node(self) -> Iterable[Project]:
yield self.config

def _generate_merged(self) -> Mapping[str, Any]:
cli_vars = self.config.cli_vars

# once sources have FQNs, add ParsedSourceDefinition
if not isinstance(self.node, (CompiledNode, ParsedNode)):
return cli_vars
search_node: IsFQNResource
if hasattr(self.node, 'fqn'):
search_node = self.node
else:
search_node = FQNLookup(self.node.package_name)

adapter_type = self.config.credentials.type

merged = MultiDict()
for project in self.packages_for_node():
merged.add(project.vars.vars_for(self.node, adapter_type))
merged.add(project.vars.vars_for(search_node, adapter_type))
merged.add(self.cli_vars)
return merged

Expand Down Expand Up @@ -494,6 +495,10 @@ class ParseProvider(Provider):
source = ParseSourceResolver


class GenerateNameProvider(ParseProvider):
Var = RuntimeVar


class RuntimeProvider(Provider):
execute = True
Config = RuntimeConfigObject
Expand Down Expand Up @@ -1120,6 +1125,17 @@ def generate_parser_macro(
return ctx.to_dict()


def generate_generate_component_name_macro(
macro: ParsedMacro,
config: RuntimeConfig,
manifest: Manifest,
) -> Dict[str, Any]:
ctx = MacroContext(
macro, config, manifest, GenerateNameProvider(), None
)
return ctx.to_dict()


def generate_runtime_model(
model: NonSourceNode,
config: RuntimeConfig,
Expand Down
17 changes: 11 additions & 6 deletions core/dbt/parser/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@

from dbt.clients.jinja import MacroGenerator
from dbt.clients.system import load_file_contents
from dbt.context.providers import generate_parser_model, generate_parser_macro
from dbt.context.providers import (
generate_parser_model,
generate_generate_component_name_macro,
)
import dbt.flags
from dbt import hooks
from dbt.adapters.factory import get_adapter
Expand Down Expand Up @@ -107,7 +110,9 @@ def __init__(
f'No macro with name generate_{component}_name found'
)

root_context = generate_parser_macro(macro, config, manifest, None)
root_context = generate_generate_component_name_macro(
macro, config, manifest
)
self.updater = MacroGenerator(macro, root_context)
self.component = component

Expand Down Expand Up @@ -324,12 +329,12 @@ def initial_config(self, fqn: List[str]) -> ContextConfigType:
config_version = min(
[self.project.config_version, self.root_project.config_version]
)
# it would be nice to assert that if the main config is v2, the
# dependencies are all v2. or vice-versa.
# grab a list of the existing project names. This is for var conversion
all_projects = self.root_project.load_dependencies()
if config_version == 1:
return LegacyContextConfig(
self.root_project.as_v1(),
self.project.as_v1(),
self.root_project.as_v1(all_projects),
self.project.as_v1(all_projects),
fqn,
self.resource_type,
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{% macro generate_schema_name(custom_schema_name, node) -%}
{% do var('somevar') %}
{% do return(dbt.generate_schema_name(custom_schema_name, node)) %}
{%- endmacro %}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
first_dep_global,from_root
dep_never_overridden,root_root_value
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
select
'{{ var("first_dep_override") }}' as first_dep_global,
'{{ var("from_root_to_root") }}' as from_root
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
first_dep_global,from_root
first_dep_global_value_overridden,root_first_value
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@

name: 'first_dep'
version: '1.0'
config-version: 2

profile: 'default'

source-paths: ["models"]
analysis-paths: ["analysis"]
test-paths: ["tests"]
data-paths: ["data"]
macro-paths: ["macros"]

require-dbt-version: '>=0.1.0'

target-path: "target" # directory which will store compiled SQL files
clean-targets: # directories to be removed by `dbt clean`
- "target"
- "dbt_modules"

vars:
first_dep:
first_dep_global: 'first_dep_global_value_overridden'


seeds:
quote_columns: False
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
select
'{{ var("first_dep_global") }}' as first_dep_global,
'{{ var("from_root_to_first") }}' as from_root
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
from_root,from_second
root_second_value,second_to_second_override_value
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@

name: 'second_dep'
version: '1.0'

profile: 'default'

source-paths: ["models"]
analysis-paths: ["analysis"]
test-paths: ["tests"]
data-paths: ["data"]
macro-paths: ["macros"]

require-dbt-version: '>=0.1.0'

target-path: "target" # directory which will store compiled SQL files
clean-targets: # directories to be removed by `dbt clean`
- "target"
- "dbt_modules"


seeds:
quote_columns: False


models:
second_dep:
vars:
from_second_to_second: 'never_see_me'
inner:
vars:
from_second_to_second: 'second_to_second_override_value'
first_dep:
vars:
from_second_to_first: 'never_see_me_either'
nested:
vars:
from_second_to_first: 'second_to_first_override_value'
test:
vars:
from_second_to_root: 'also_never_see_me'
inside:
vars:
from_second_to_root: 'second_to_root_override_value'
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
select
'{{ var("from_root_to_second") }}' as from_root,
'{{ var("from_second_to_second") }}' as from_second
Loading

0 comments on commit e2cd45b

Please sign in to comment.