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

Fix some issues around global-scoped vars #2477

Merged
merged 2 commits into from
May 21, 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
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the thinking here? Is it that we don't want to provide package-scoped vars to the wrong package? It also means that you can't provide a global var that is a dict to a v1 package, right?

I'm curious what the ramifications would be if we removed this filter and added all of the src_vars to the v1 package's vars

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. We can remove that if not isinstance, I thought it made the debug dictionary really a pain.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, that's fine - let's ship it as-is and I'll be sure to update the docs accordingly

})

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
39 changes: 30 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 isinstance(self.node, IsFQNResource):
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,15 @@ class ParseProvider(Provider):
source = ParseSourceResolver


class GenerateNameProvider(Provider):
execute = False
Config = RuntimeConfigObject
DatabaseWrapper = ParseDatabaseWrapper
Var = RuntimeVar
ref = ParseRefResolver
source = ParseSourceResolver


class RuntimeProvider(Provider):
execute = True
Config = RuntimeConfigObject
Expand Down Expand Up @@ -1120,6 +1130,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
3 changes: 2 additions & 1 deletion core/dbt/legacy_config_updater.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# TODO: rename this module.
from typing import Dict, Any, Mapping, List
from typing_extensions import Protocol
from typing_extensions import Protocol, runtime_checkable

import dbt.exceptions

Expand All @@ -16,6 +16,7 @@ class HasConfigFields(Protocol):
sources: Dict[str, Any]


@runtime_checkable
class IsFQNResource(Protocol):
fqn: List[str]
resource_type: NodeType
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
12 changes: 8 additions & 4 deletions core/dbt/rpc/builtins.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import os
import signal
from datetime import datetime
from typing import Type, Union, Any, List
from typing import Type, Union, Any, List, Dict

import dbt.exceptions
from dbt.contracts.rpc import (
Expand Down Expand Up @@ -171,6 +171,10 @@ def poll_complete(
return cls.from_result(result, tags, timing, logs)


def _dict_logs(logs: List[LogMessage]) -> List[Dict[str, Any]]:
return [log.to_dict() for log in logs]


class Poll(RemoteBuiltinMethod[PollParameters, PollResult]):
METHOD_NAME = 'poll'

Expand Down Expand Up @@ -210,7 +214,7 @@ def handle_request(self) -> PollResult:
f'At end of task {task_id}, error state but error is None'
)
raise RPCException.from_error(
dbt_error(exc, logs=[l.to_dict() for l in task_logs])
dbt_error(exc, logs=_dict_logs(task_logs))
)
# the exception has logs already attached from the child, don't
# overwrite those
Expand All @@ -223,7 +227,7 @@ def handle_request(self) -> PollResult:
'None'
)
raise RPCException.from_error(
dbt_error(exc, logs=[l.to_dict() for l in task_logs])
dbt_error(exc, logs=_dict_logs(task_logs))
)
return poll_complete(
timing=timing,
Expand All @@ -245,5 +249,5 @@ def handle_request(self) -> PollResult:
f'Got unknown value state={state} for task {task_id}'
)
raise RPCException.from_error(
dbt_error(exc, logs=[l.to_dict() for l in task_logs])
dbt_error(exc, logs=_dict_logs(task_logs))
)
2 changes: 1 addition & 1 deletion core/dbt/rpc/task_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ def get_result(self) -> RemoteResult:
except RPCException as exc:
# RPC Exceptions come already preserialized for the jsonrpc
# framework
exc.logs = [l.to_dict() for l in self.logs]
exc.logs = [log.to_dict() for log in self.logs]
exc.tags = self.tags
raise

Expand Down
10 changes: 0 additions & 10 deletions core/dbt/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,23 +36,13 @@ class ExitCodes(int, Enum):
UnhandledError = 2


def to_bytes(s):
return s.encode('latin-1')


def coalesce(*args):
for arg in args:
if arg is not None:
return arg
return None


def chunks(l, n):
"""Yield successive n-sized chunks from l."""
for i in range(0, len(l), n):
yield l[i:i + n]


def get_profile_from_project(project):
target_name = project.get('target', {})
profile = project.get('outputs', {}).get(target_name, {})
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
Loading