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

[#3885] Skip partial parsing if project env vars change #4212

Merged
merged 2 commits into from
Nov 8, 2021
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 @@ -52,6 +52,7 @@ Contributors:
- Dependency updates ([#4079](https://github.com/dbt-labs/dbt-core/pull/4079)), ([#3532](https://github.com/dbt-labs/dbt-core/pull/3532)
- Schedule partial parsing for SQL files with env_var changes ([#3885](https://github.com/dbt-labs/dbt-core/issues/3885), [#4101](https://github.com/dbt-labs/dbt-core/pull/4101))
- Schedule partial parsing for schema files with env_var changes ([#3885](https://github.com/dbt-labs/dbt-core/issues/3885), [#4162](https://github.com/dbt-labs/dbt-core/pull/4162))
- Skip partial parsing when env_vars change in dbt_project or profile ([#3885](https://github.com/dbt-labs/dbt-core/issues/3885), [#4212](https://github.com/dbt-labs/dbt-core/pull/4212))

Contributors:
- [@sungchun12](https://github.com/sungchun12) ([#4017](https://github.com/dbt-labs/dbt/pull/4017))
Expand Down
2 changes: 2 additions & 0 deletions core/dbt/config/profile.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ class Profile(HasCredentials):
user_config: UserConfig
threads: int
credentials: Credentials
profile_env_vars: Dict[str, Any]

def __init__(
self,
Expand All @@ -108,6 +109,7 @@ def __init__(
self.user_config = user_config
self.threads = threads
self.credentials = credentials
self.profile_env_vars = {} # never available on init

def to_profile_info(
self, serialize_credentials: bool = False
Expand Down
25 changes: 5 additions & 20 deletions core/dbt/config/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ def get_rendered(
selectors_dict=rendered_selectors,
)

# Called by 'collect_parts' in RuntimeConfig
def render(self, renderer: DbtProjectYamlRenderer) -> 'Project':
try:
rendered = self.get_rendered(renderer)
Expand Down Expand Up @@ -397,6 +398,8 @@ def create_project(self, rendered: RenderComponents) -> 'Project':
vars_dict = cfg.vars

vars_value = VarProvider(vars_dict)
# There will never be any project_env_vars when it's first created
project_env_vars: Dict[str, Any] = {}
Comment on lines +401 to +402
Copy link
Member

@emmyoop emmyoop Nov 8, 2021

Choose a reason for hiding this comment

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

Is this true? With the new interactive init you can technically put in env_vars. Either way they're "changing" since they're new so not sure how much it matters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. The project_env_vars only come from rendering, which happens after the object is created. So whether or not env vars exist before the dbt code starts up doesn't matter.

on_run_start: List[str] = value_or(cfg.on_run_start, [])
on_run_end: List[str] = value_or(cfg.on_run_end, [])

Expand Down Expand Up @@ -444,6 +447,7 @@ def create_project(self, rendered: RenderComponents) -> 'Project':
vars=vars_value,
config_version=cfg.config_version,
unrendered=unrendered,
project_env_vars=project_env_vars,
)
# sanity check - this means an internal issue
project.validate()
Expand Down Expand Up @@ -556,6 +560,7 @@ class Project:
query_comment: QueryComment
config_version: int
unrendered: RenderComponents
project_env_vars: Dict[str, Any]

@property
def all_source_paths(self) -> List[str]:
Expand Down Expand Up @@ -645,26 +650,6 @@ def partial_load(
verify_version=verify_version,
)

@classmethod
def render_from_dict(
cls,
project_root: str,
project_dict: Dict[str, Any],
packages_dict: Dict[str, Any],
selectors_dict: Dict[str, Any],
renderer: DbtProjectYamlRenderer,
*,
verify_version: bool = False
) -> 'Project':
partial = PartialProject.from_dicts(
project_root=project_root,
project_dict=project_dict,
packages_dict=packages_dict,
selectors_dict=selectors_dict,
verify_version=verify_version,
)
return partial.render(renderer)

@classmethod
def from_project_root(
cls,
Expand Down
33 changes: 32 additions & 1 deletion core/dbt/config/renderer.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
from typing import Dict, Any, Tuple, Optional, Union, Callable

from dbt.clients.jinja import get_rendered, catch_jinja

from dbt.context.target import TargetContext
from dbt.context.base import BaseContext
from dbt.contracts.connection import HasCredentials
from dbt.exceptions import (
DbtProjectError, CompilationException, RecursionException
)
Expand Down Expand Up @@ -98,6 +100,23 @@ def postprocess(self, value: Any, key: Keypath) -> Any:
class DbtProjectYamlRenderer(BaseRenderer):
_KEYPATH_HANDLERS = ProjectPostprocessor()

def __init__(
self, profile: Optional[HasCredentials] = None,
cli_vars: Optional[Dict[str, Any]] = None
) -> None:
# Generate contexts here because we want to save the context
# object in order to retrieve the env_vars. This is almost always
# a TargetContext, but in the debug task we want a project
# even when we don't have a profile.
if cli_vars is None:
cli_vars = {}
if profile:
self.ctx_obj = TargetContext(profile, cli_vars)
else:
self.ctx_obj = BaseContext(cli_vars) # type:ignore
context = self.ctx_obj.to_dict()
super().__init__(context)

@property
def name(self):
'Project config'
Expand Down Expand Up @@ -157,6 +176,18 @@ def should_render_keypath(self, keypath: Keypath) -> bool:


class ProfileRenderer(BaseRenderer):

def __init__(
self, cli_vars: Optional[Dict[str, Any]] = None
) -> None:
# Generate contexts here because we want to save the context
# object in order to retrieve the env_vars.
if cli_vars is None:
cli_vars = {}
self.ctx_obj = BaseContext(cli_vars)
context = self.ctx_obj.to_dict()
super().__init__(context)

@property
def name(self):
'Profile'
Expand Down
30 changes: 20 additions & 10 deletions core/dbt/config/runtime.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@
from dbt import tracking
from dbt.adapters.factory import get_relation_class_by_name, get_include_paths
from dbt.helper_types import FQNPath, PathSet
from dbt.context.base import generate_base_context
from dbt.context.target import generate_target_context
from dbt.contracts.connection import AdapterRequiredConfig, Credentials
from dbt.contracts.graph.manifest import ManifestMetadata
from dbt.contracts.relation import ComponentName
Expand Down Expand Up @@ -60,6 +58,7 @@ class RuntimeConfig(Project, Profile, AdapterRequiredConfig):
def __post_init__(self):
self.validate()

# Called by 'new_project' and 'from_args'
@classmethod
def from_parts(
cls,
Expand Down Expand Up @@ -116,6 +115,8 @@ def from_parts(
vars=project.vars,
config_version=project.config_version,
unrendered=project.unrendered,
project_env_vars=project.project_env_vars,
profile_env_vars=profile.profile_env_vars,
profile_name=profile.profile_name,
target_name=profile.target_name,
user_config=profile.user_config,
Expand All @@ -126,6 +127,7 @@ def from_parts(
dependencies=dependencies,
)

# Called by 'load_projects' in this class
def new_project(self, project_root: str) -> 'RuntimeConfig':
"""Given a new project root, read in its project dictionary, supply the
existing project's profile info, and create a new project file.
Expand All @@ -140,22 +142,22 @@ def new_project(self, project_root: str) -> 'RuntimeConfig':
profile.validate()

# load the new project and its packages. Don't pass cli variables.
renderer = DbtProjectYamlRenderer(generate_target_context(profile, {}))
renderer = DbtProjectYamlRenderer(profile)

project = Project.from_project_root(
project_root,
renderer,
verify_version=bool(flags.VERSION_CHECK),
)

cfg = self.from_parts(
runtime_config = self.from_parts(
project=project,
profile=profile,
args=deepcopy(self.args),
)
# force our quoting back onto the new project.
cfg.quoting = deepcopy(self.quoting)
return cfg
runtime_config.quoting = deepcopy(self.quoting)
return runtime_config

def serialize(self) -> Dict[str, Any]:
"""Serialize the full configuration to a single dictionary. For any
Expand Down Expand Up @@ -205,21 +207,26 @@ def collect_parts(
)

# build the profile using the base renderer and the one fact we know
# Note: only the named profile section is rendered. The rest of the
# profile is ignored.
cli_vars: Dict[str, Any] = parse_cli_vars(getattr(args, 'vars', '{}'))
profile_renderer = ProfileRenderer(generate_base_context(cli_vars))
profile_renderer = ProfileRenderer(cli_vars)
profile_name = partial.render_profile_name(profile_renderer)

profile = cls._get_rendered_profile(
args, profile_renderer, profile_name
)
# Save env_vars encountered in rendering for partial parsing
profile.profile_env_vars = profile_renderer.ctx_obj.env_vars

# get a new renderer using our target information and render the
# project
ctx = generate_target_context(profile, cli_vars)
project_renderer = DbtProjectYamlRenderer(ctx)
project_renderer = DbtProjectYamlRenderer(profile, cli_vars)
project = partial.render(project_renderer)
# Save env_vars encountered in rendering for partial parsing
project.project_env_vars = project_renderer.ctx_obj.env_vars
return (project, profile)

# Called in main.py, lib.py, task/base.py
@classmethod
def from_args(cls, args: Any) -> 'RuntimeConfig':
"""Given arguments, read in dbt_project.yml from the current directory,
Expand Down Expand Up @@ -360,6 +367,7 @@ def load_dependencies(self) -> Mapping[str, 'RuntimeConfig']:
def clear_dependencies(self):
self.dependencies = None

# Called by 'load_dependencies' in this class
def load_projects(
self, paths: Iterable[Path]
) -> Iterator[Tuple[str, 'RuntimeConfig']]:
Expand Down Expand Up @@ -512,6 +520,8 @@ def from_parts(
vars=project.vars,
config_version=project.config_version,
unrendered=project.unrendered,
project_env_vars=project.project_env_vars,
profile_env_vars=profile.profile_env_vars,
profile_name='',
target_name='',
user_config=UnsetConfig(),
Expand Down
2 changes: 2 additions & 0 deletions core/dbt/context/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ class BaseContext(metaclass=ContextMeta):
def __init__(self, cli_vars):
self._ctx = {}
self.cli_vars = cli_vars
self.env_vars = {}

def generate_builtins(self):
builtins: Dict[str, Any] = {}
Expand Down Expand Up @@ -317,6 +318,7 @@ def env_var(self, var: str, default: Optional[str] = None) -> str:
return_value = default

if return_value is not None:
self.env_vars[var] = return_value
return return_value
else:
msg = f"Env var required but not provided: '{var}'"
Expand Down
2 changes: 2 additions & 0 deletions core/dbt/contracts/graph/manifest.py
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,8 @@ class ParsingInfo:
@dataclass
class ManifestStateCheck(dbtClassMixin):
vars_hash: FileHash = field(default_factory=FileHash.empty)
project_env_vars_hash: FileHash = field(default_factory=FileHash.empty)
profile_env_vars_hash: FileHash = field(default_factory=FileHash.empty)
profile_hash: FileHash = field(default_factory=FileHash.empty)
project_hashes: MutableMapping[str, FileHash] = field(default_factory=dict)

Expand Down
4 changes: 1 addition & 3 deletions core/dbt/deps/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

from dbt.exceptions import raise_dependency_error, InternalException

from dbt.context.target import generate_target_context
from dbt.config import Project, RuntimeConfig
from dbt.config.renderer import DbtProjectYamlRenderer
from dbt.deps.base import BasePackage, PinnedPackage, UnpinnedPackage
Expand Down Expand Up @@ -126,8 +125,7 @@ def resolve_packages(
pending = PackageListing.from_contracts(packages)
final = PackageListing()

ctx = generate_target_context(config, config.cli_vars)
renderer = DbtProjectYamlRenderer(ctx)
renderer = DbtProjectYamlRenderer(config, config.cli_vars)

while pending:
next_pending = PackageListing()
Expand Down
Loading