From 8c4b97db22567196182763ad4e3f9d543a78fd7e Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Thu, 4 Nov 2021 11:23:05 -0400 Subject: [PATCH 1/2] [#3885] Skip partial parsing if project env vars change --- core/dbt/config/project.py | 25 ++------- core/dbt/config/renderer.py | 21 +++++++- core/dbt/config/runtime.py | 19 ++++--- core/dbt/context/base.py | 2 + core/dbt/contracts/graph/manifest.py | 1 + core/dbt/deps/resolver.py | 4 +- core/dbt/parser/manifest.py | 32 ++++++------ core/dbt/task/base.py | 2 + core/dbt/task/debug.py | 12 +---- core/dbt/task/deps.py | 5 +- .../068_partial_parsing_tests/test_pp_vars.py | 51 +++++++++++++++++++ test/unit/test_config.py | 5 +- test/unit/test_graph.py | 9 +--- test/unit/utils.py | 14 +++-- 14 files changed, 127 insertions(+), 75 deletions(-) diff --git a/core/dbt/config/project.py b/core/dbt/config/project.py index 8acc27e7338..c5c98f63ee3 100644 --- a/core/dbt/config/project.py +++ b/core/dbt/config/project.py @@ -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) @@ -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] = {} on_run_start: List[str] = value_or(cfg.on_run_start, []) on_run_end: List[str] = value_or(cfg.on_run_end, []) @@ -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() @@ -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]: @@ -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, diff --git a/core/dbt/config/renderer.py b/core/dbt/config/renderer.py index 5c27439eb1a..9376593055b 100644 --- a/core/dbt/config/renderer.py +++ b/core/dbt/config/renderer.py @@ -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 ) @@ -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' diff --git a/core/dbt/config/runtime.py b/core/dbt/config/runtime.py index bd86d7df084..bec2a22eff9 100644 --- a/core/dbt/config/runtime.py +++ b/core/dbt/config/runtime.py @@ -17,7 +17,6 @@ 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 @@ -60,6 +59,7 @@ class RuntimeConfig(Project, Profile, AdapterRequiredConfig): def __post_init__(self): self.validate() + # Called by 'new_project' and 'from_args' @classmethod def from_parts( cls, @@ -116,6 +116,7 @@ def from_parts( vars=project.vars, config_version=project.config_version, unrendered=project.unrendered, + project_env_vars=project.project_env_vars, profile_name=profile.profile_name, target_name=profile.target_name, user_config=profile.user_config, @@ -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. @@ -140,7 +142,7 @@ 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, @@ -148,14 +150,14 @@ def new_project(self, project_root: str) -> 'RuntimeConfig': 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 @@ -215,11 +217,12 @@ def collect_parts( # 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) + 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, @@ -360,6 +363,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']]: @@ -512,6 +516,7 @@ def from_parts( vars=project.vars, config_version=project.config_version, unrendered=project.unrendered, + project_env_vars=project.project_env_vars, profile_name='', target_name='', user_config=UnsetConfig(), diff --git a/core/dbt/context/base.py b/core/dbt/context/base.py index c731ce8ec0d..4bc47000716 100644 --- a/core/dbt/context/base.py +++ b/core/dbt/context/base.py @@ -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] = {} @@ -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}'" diff --git a/core/dbt/contracts/graph/manifest.py b/core/dbt/contracts/graph/manifest.py index ad2e75e6760..55d13415901 100644 --- a/core/dbt/contracts/graph/manifest.py +++ b/core/dbt/contracts/graph/manifest.py @@ -546,6 +546,7 @@ class ParsingInfo: @dataclass class ManifestStateCheck(dbtClassMixin): vars_hash: FileHash = field(default_factory=FileHash.empty) + 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) diff --git a/core/dbt/deps/resolver.py b/core/dbt/deps/resolver.py index 31dcad63645..6e1b6f126f2 100644 --- a/core/dbt/deps/resolver.py +++ b/core/dbt/deps/resolver.py @@ -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 @@ -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() diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index f62ad02dfd8..709261af8bf 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -78,6 +78,7 @@ class ReparseReason(StrEnum): project_config_changed = '06_project_config_changed' load_file_failure = '07_load_file_failure' exception = '08_exception' + env_vars_changed = '09_env_vars_changed' # Part of saved performance info @@ -553,6 +554,10 @@ def is_partial_parsable(self, manifest: Manifest) -> Tuple[bool, Optional[str]]: logger.info("Unable to do partial parsing because profile has changed") valid = False reparse_reason = ReparseReason.profile_changed + if self.manifest.state_check.env_vars_hash != manifest.state_check.env_vars_hash: + logger.info("Unable to do partial parsing because env vars have changed") + valid = False + reparse_reason = ReparseReason.env_vars_changed missing_keys = { k for k in self.manifest.state_check.project_hashes @@ -605,8 +610,8 @@ def read_manifest_for_partial_parse(self) -> Optional[Manifest]: # keep this check inside the try/except in case something about # the file has changed in weird ways, perhaps due to being a # different version of dbt - is_partial_parseable, reparse_reason = self.is_partial_parsable(manifest) - if is_partial_parseable: + is_partial_parsable, reparse_reason = self.is_partial_parsable(manifest) + if is_partial_parsable: # We don't want to have stale generated_at dates manifest.metadata.generated_at = datetime.utcnow() # or invocation_ids @@ -664,6 +669,14 @@ def build_manifest_state_check(self): ]) ) + # Create a hash of the env_vars in the project + key_list = list(config.project_env_vars.keys()) + key_list.sort() + env_var_str = '' + for key in key_list: + env_var_str = env_var_str + f'{key}:{config.project_env_vars[key]}|' + env_vars_hash = FileHash.from_contents(env_var_str) + profile_path = os.path.join(flags.PROFILES_DIR, 'profiles.yml') with open(profile_path) as fp: profile_hash = FileHash.from_contents(fp.read()) @@ -675,6 +688,7 @@ def build_manifest_state_check(self): project_hashes[name] = FileHash.from_contents(fp.read()) state_check = ManifestStateCheck( + env_vars_hash=env_vars_hash, vars_hash=vars_hash, profile_hash=profile_hash, project_hashes=project_hashes, @@ -923,20 +937,6 @@ def _check_manifest(manifest: Manifest, config: RuntimeConfig) -> None: _warn_for_unused_resource_config_paths(manifest, config) -# This is just used in test cases -def _load_projects(config, paths): - for path in paths: - try: - project = config.new_project(path) - except dbt.exceptions.DbtProjectError as e: - raise dbt.exceptions.DbtProjectError( - 'Failed to read package at {}: {}' - .format(path, e) - ) - else: - yield project.project_name, project - - def _get_node_column(node, column_name): """Given a ParsedNode, add some fields that might be missing. Return a reference to the dict that refers to the given column, creating it if diff --git a/core/dbt/task/base.py b/core/dbt/task/base.py index af0eb3de818..5b4b6ab594d 100644 --- a/core/dbt/task/base.py +++ b/core/dbt/task/base.py @@ -80,6 +80,8 @@ def set_log_format(cls): @classmethod def from_args(cls, args): try: + # This is usually RuntimeConfig but will be UnsetProfileConfig + # for the clean or deps tasks config = cls.ConfigType.from_args(args) except dbt.exceptions.DbtProjectError as exc: logger.error("Encountered an error while reading the project:") diff --git a/core/dbt/task/debug.py b/core/dbt/task/debug.py index c26c2ad0f39..d29b35e9cc4 100644 --- a/core/dbt/task/debug.py +++ b/core/dbt/task/debug.py @@ -13,7 +13,6 @@ from dbt.config.renderer import DbtProjectYamlRenderer, ProfileRenderer from dbt.config.utils import parse_cli_vars from dbt.context.base import generate_base_context -from dbt.context.target import generate_target_context from dbt.clients.yaml_helper import load_yaml_text from dbt.links import ProfileConfigDocs from dbt.ui import green, red @@ -146,12 +145,7 @@ def _load_project(self): self.project_fail_details = FILE_NOT_FOUND return red('ERROR not found') - if self.profile is None: - ctx = generate_base_context(self.cli_vars) - else: - ctx = generate_target_context(self.profile, self.cli_vars) - - renderer = DbtProjectYamlRenderer(ctx) + renderer = DbtProjectYamlRenderer(self.profile, self.cli_vars) try: self.project = Project.from_project_root( @@ -198,9 +192,7 @@ def _choose_profile_names(self) -> Optional[List[str]]: os.path.dirname(self.project_path), verify_version=bool(flags.VERSION_CHECK), ) - renderer = DbtProjectYamlRenderer( - generate_base_context(self.cli_vars) - ) + renderer = DbtProjectYamlRenderer(None, self.cli_vars) project_profile = partial.render_profile_name(renderer) except dbt.exceptions.DbtProjectError: pass diff --git a/core/dbt/task/deps.py b/core/dbt/task/deps.py index 573098a8080..04992bd550f 100644 --- a/core/dbt/task/deps.py +++ b/core/dbt/task/deps.py @@ -4,7 +4,6 @@ from dbt.config import UnsetProfileConfig from dbt.config.renderer import DbtProjectYamlRenderer -from dbt.context.target import generate_target_context from dbt.deps.base import downloads_directory from dbt.deps.resolver import resolve_packages @@ -52,9 +51,7 @@ def run(self): with downloads_directory(): final_deps = resolve_packages(packages, self.config) - renderer = DbtProjectYamlRenderer(generate_target_context( - self.config, self.config.cli_vars - )) + renderer = DbtProjectYamlRenderer(self.config, self.config.cli_vars) packages_to_upgrade = [] for package in final_deps: diff --git a/test/integration/068_partial_parsing_tests/test_pp_vars.py b/test/integration/068_partial_parsing_tests/test_pp_vars.py index dbb77f49021..e9e4779cf5d 100644 --- a/test/integration/068_partial_parsing_tests/test_pp_vars.py +++ b/test/integration/068_partial_parsing_tests/test_pp_vars.py @@ -229,4 +229,55 @@ def test_postgres_env_vars_models(self): del os.environ['TEST_SCHEMA_VAR'] del os.environ['ENV_VAR_COLOR'] del os.environ['ENV_VAR_SOME_KEY'] + del os.environ['ENV_VAR_OWNER'] + +class ProjectEnvVarTest(BasePPTest): + + @property + def project_config(self): + # Need to set the environment variable here initially because + # the unittest setup does a load_config. + os.environ['ENV_VAR_NAME'] = "Jane Smith" + return { + 'config-version': 2, + 'seed-paths': ['seeds'], + 'test-paths': ['tests'], + 'macro-paths': ['macros'], + 'seeds': { + 'quote_columns': False, + }, + 'models': { + '+meta': { + 'meta_name': "{{ env_var('ENV_VAR_NAME') }}" + } + } + } + + @use_profile('postgres') + def test_postgres_project_env_vars(self): + + # Initial run + self.setup_directories() + self.copy_file('test-files/model_one.sql', 'models/model_one.sql') + self.run_dbt(['clean']) + results = self.run_dbt(["run"]) + self.assertEqual(len(results), 1) + manifest = get_manifest() + state_check = manifest.state_check + model_id = 'model.test.model_one' + model = manifest.nodes[model_id] + self.assertEqual(model.config.meta['meta_name'], 'Jane Smith') + env_vars_hash_checksum = state_check.env_vars_hash.checksum + + # Change the environment variable + os.environ['ENV_VAR_NAME'] = "Jane Doe" + results = self.run_dbt(["run"]) + self.assertEqual(len(results), 1) + manifest = get_manifest() + model = manifest.nodes[model_id] + self.assertEqual(model.config.meta['meta_name'], 'Jane Doe') + self.assertNotEqual(env_vars_hash_checksum, manifest.state_check.env_vars_hash.checksum) + + # cleanup + del os.environ['ENV_VAR_NAME'] diff --git a/test/unit/test_config.py b/test/unit/test_config.py index 51ee046ef4f..c9e509fa622 100644 --- a/test/unit/test_config.py +++ b/test/unit/test_config.py @@ -47,7 +47,7 @@ def empty_profile_renderer(): def empty_project_renderer(): - return dbt.config.renderer.DbtProjectYamlRenderer(generate_base_context({})) + return dbt.config.renderer.DbtProjectYamlRenderer() model_config = { @@ -943,13 +943,14 @@ def setUp(self): self.default_project_data['project-root'] = self.project_dir def test_cli_and_env_vars(self): - renderer = dbt.config.renderer.DbtProjectYamlRenderer(generate_base_context({'cli_version': '0.1.2'})) + renderer = dbt.config.renderer.DbtProjectYamlRenderer(None, {'cli_version': '0.1.2'}) with mock.patch.dict(os.environ, self.env_override): project = dbt.config.Project.from_project_root( self.project_dir, renderer, ) + self.assertEqual(renderer.ctx_obj.env_vars, {'env_value_profile': 'default'}) self.assertEqual(project.version, "0.1.2") self.assertEqual(project.project_name, 'blah') self.assertEqual(project.profile_name, 'default') diff --git a/test/unit/test_graph.py b/test/unit/test_graph.py index b73a8010511..7a26a9cbbeb 100644 --- a/test/unit/test_graph.py +++ b/test/unit/test_graph.py @@ -31,7 +31,6 @@ class GraphTest(unittest.TestCase): def tearDown(self): self.write_gpickle_patcher.stop() - self.load_projects_patcher.stop() self.file_system_patcher.stop() self.mock_filesystem_constructor.stop() self.mock_hook_constructor.stop() @@ -97,19 +96,13 @@ def create_hook_patcher(cls, project, manifest, root_project): self.mock_hook_constructor = self.hook_patcher.start() self.mock_hook_constructor.side_effect = create_hook_patcher - # Create load_projects patcher - self.load_projects_patcher = patch('dbt.parser.manifest._load_projects') - self.mock_load_projects = self.load_projects_patcher.start() - def _load_projects(config, paths): - yield config.project_name, config - self.mock_load_projects.side_effect = _load_projects - # Create the Manifest.state_check patcher @patch('dbt.parser.manifest.ManifestLoader.build_manifest_state_check') def _mock_state_check(self): config = self.root_project all_projects = self.all_projects return ManifestStateCheck( + env_vars_hash=FileHash.from_contents(''), vars_hash=FileHash.from_contents('vars'), project_hashes={name: FileHash.from_contents(name) for name in all_projects}, profile_hash=FileHash.from_contents('profile'), diff --git a/test/unit/utils.py b/test/unit/utils.py index 051f4a4e36d..3b3ed324507 100644 --- a/test/unit/utils.py +++ b/test/unit/utils.py @@ -11,6 +11,7 @@ import agate import pytest from dbt.dataclass_schema import ValidationError +from dbt.config.project import PartialProject def normalize(path): @@ -62,13 +63,18 @@ def project_from_dict(project, profile, packages=None, selectors=None, cli_vars= if not isinstance(cli_vars, dict): cli_vars = parse_cli_vars(cli_vars) - renderer = DbtProjectYamlRenderer(generate_target_context(profile, cli_vars)) + renderer = DbtProjectYamlRenderer(profile, cli_vars) project_root = project.pop('project-root', os.getcwd()) - return Project.render_from_dict( - project_root, project, packages, selectors, renderer - ) + partial = PartialProject.from_dicts( + project_root=project_root, + project_dict=project, + packages_dict=packages, + selectors_dict=selectors, + ) + return partial.render(renderer) + def config_from_parts_or_dicts(project, profile, packages=None, selectors=None, cli_vars='{}'): From c449da133b1561a51dfad4e87da7df6944b3a0cb Mon Sep 17 00:00:00 2001 From: Gerda Shank Date: Fri, 5 Nov 2021 13:27:39 -0400 Subject: [PATCH 2/2] Support env_vars in the profile --- CHANGELOG.md | 1 + core/dbt/config/profile.py | 2 + core/dbt/config/renderer.py | 12 ++++ core/dbt/config/runtime.py | 11 +++- core/dbt/contracts/graph/manifest.py | 3 +- core/dbt/parser/manifest.py | 41 ++++++++++--- core/dbt/task/debug.py | 7 +-- .../068_partial_parsing_tests/test_pp_vars.py | 57 ++++++++++++++++++- test/unit/test_config.py | 4 +- test/unit/test_graph.py | 3 +- test/unit/utils.py | 2 +- 11 files changed, 122 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d161f099c9..be52475eda4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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)) diff --git a/core/dbt/config/profile.py b/core/dbt/config/profile.py index fc6213258f7..1800111652e 100644 --- a/core/dbt/config/profile.py +++ b/core/dbt/config/profile.py @@ -91,6 +91,7 @@ class Profile(HasCredentials): user_config: UserConfig threads: int credentials: Credentials + profile_env_vars: Dict[str, Any] def __init__( self, @@ -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 diff --git a/core/dbt/config/renderer.py b/core/dbt/config/renderer.py index 9376593055b..e413a51ce19 100644 --- a/core/dbt/config/renderer.py +++ b/core/dbt/config/renderer.py @@ -176,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' diff --git a/core/dbt/config/runtime.py b/core/dbt/config/runtime.py index bec2a22eff9..a0857d94bf3 100644 --- a/core/dbt/config/runtime.py +++ b/core/dbt/config/runtime.py @@ -16,7 +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.contracts.connection import AdapterRequiredConfig, Credentials from dbt.contracts.graph.manifest import ManifestMetadata from dbt.contracts.relation import ComponentName @@ -117,6 +116,7 @@ def from_parts( 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, @@ -207,18 +207,22 @@ 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 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) @@ -517,6 +521,7 @@ def from_parts( 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(), diff --git a/core/dbt/contracts/graph/manifest.py b/core/dbt/contracts/graph/manifest.py index 55d13415901..2fbe0f30ed8 100644 --- a/core/dbt/contracts/graph/manifest.py +++ b/core/dbt/contracts/graph/manifest.py @@ -546,7 +546,8 @@ class ParsingInfo: @dataclass class ManifestStateCheck(dbtClassMixin): vars_hash: FileHash = field(default_factory=FileHash.empty) - env_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) diff --git a/core/dbt/parser/manifest.py b/core/dbt/parser/manifest.py index 709261af8bf..bc679dec2de 100644 --- a/core/dbt/parser/manifest.py +++ b/core/dbt/parser/manifest.py @@ -78,7 +78,8 @@ class ReparseReason(StrEnum): project_config_changed = '06_project_config_changed' load_file_failure = '07_load_file_failure' exception = '08_exception' - env_vars_changed = '09_env_vars_changed' + proj_env_vars_changed = '09_project_env_vars_changed' + prof_env_vars_changed = '10_profile_env_vars_changed' # Part of saved performance info @@ -554,10 +555,18 @@ def is_partial_parsable(self, manifest: Manifest) -> Tuple[bool, Optional[str]]: logger.info("Unable to do partial parsing because profile has changed") valid = False reparse_reason = ReparseReason.profile_changed - if self.manifest.state_check.env_vars_hash != manifest.state_check.env_vars_hash: - logger.info("Unable to do partial parsing because env vars have changed") + if self.manifest.state_check.project_env_vars_hash != \ + manifest.state_check.project_env_vars_hash: + logger.info("Unable to do partial parsing because env vars " + "used in dbt_project.yml have changed") valid = False - reparse_reason = ReparseReason.env_vars_changed + reparse_reason = ReparseReason.proj_env_vars_changed + if self.manifest.state_check.profile_env_vars_hash != \ + manifest.state_check.profile_env_vars_hash: + logger.info("Unable to do partial parsing because env vars " + "used in profiles.yml have changed") + valid = False + reparse_reason = ReparseReason.prof_env_vars_changed missing_keys = { k for k in self.manifest.state_check.project_hashes @@ -660,6 +669,12 @@ def build_manifest_state_check(self): config = self.root_project all_projects = self.all_projects # if any of these change, we need to reject the parser + + # Create a FileHash of vars string, profile name and target name + # This does not capture vars in dbt_project, just the command line + # arg vars, but since any changes to that file will cause state_check + # to not pass, it doesn't matter. If we move to more granular checking + # of env_vars, that would need to change. vars_hash = FileHash.from_contents( '\x00'.join([ getattr(config.args, 'vars', '{}') or '{}', @@ -669,26 +684,38 @@ def build_manifest_state_check(self): ]) ) - # Create a hash of the env_vars in the project + # Create a FileHash of the env_vars in the project key_list = list(config.project_env_vars.keys()) key_list.sort() env_var_str = '' for key in key_list: env_var_str = env_var_str + f'{key}:{config.project_env_vars[key]}|' - env_vars_hash = FileHash.from_contents(env_var_str) + project_env_vars_hash = FileHash.from_contents(env_var_str) + + # Create a FileHash of the env_vars in the project + key_list = list(config.profile_env_vars.keys()) + key_list.sort() + env_var_str = '' + for key in key_list: + env_var_str = env_var_str + f'{key}:{config.profile_env_vars[key]}|' + profile_env_vars_hash = FileHash.from_contents(env_var_str) + # Create a FileHash of the profile file profile_path = os.path.join(flags.PROFILES_DIR, 'profiles.yml') with open(profile_path) as fp: profile_hash = FileHash.from_contents(fp.read()) + # Create a FileHashes for dbt_project for all dependencies project_hashes = {} for name, project in all_projects.items(): path = os.path.join(project.project_root, 'dbt_project.yml') with open(path) as fp: project_hashes[name] = FileHash.from_contents(fp.read()) + # Create the ManifestStateCheck object state_check = ManifestStateCheck( - env_vars_hash=env_vars_hash, + project_env_vars_hash=project_env_vars_hash, + profile_env_vars_hash=profile_env_vars_hash, vars_hash=vars_hash, profile_hash=profile_hash, project_hashes=project_hashes, diff --git a/core/dbt/task/debug.py b/core/dbt/task/debug.py index d29b35e9cc4..92c6f93e88f 100644 --- a/core/dbt/task/debug.py +++ b/core/dbt/task/debug.py @@ -12,7 +12,6 @@ from dbt.config import Project, Profile from dbt.config.renderer import DbtProjectYamlRenderer, ProfileRenderer from dbt.config.utils import parse_cli_vars -from dbt.context.base import generate_base_context from dbt.clients.yaml_helper import load_yaml_text from dbt.links import ProfileConfigDocs from dbt.ui import green, red @@ -233,7 +232,7 @@ def _choose_target_name(self, profile_name: str): assert self.raw_profile_data is not None raw_profile = self.raw_profile_data[profile_name] - renderer = ProfileRenderer(generate_base_context(self.cli_vars)) + renderer = ProfileRenderer(self.cli_vars) target_name, _ = Profile.render_profile( raw_profile=raw_profile, @@ -264,7 +263,7 @@ def _load_profile(self): profile_errors = [] profile_names = self._choose_profile_names() - renderer = ProfileRenderer(generate_base_context(self.cli_vars)) + renderer = ProfileRenderer(self.cli_vars) for profile_name in profile_names: try: profile: Profile = QueryCommentedProfile.render_from_args( @@ -392,7 +391,7 @@ def validate_connection(cls, target_dict): raw_profile=profile_data, profile_name='', target_override=target_name, - renderer=ProfileRenderer(generate_base_context({})), + renderer=ProfileRenderer({}), ) result = cls.attempt_connection(profile) if result is not None: diff --git a/test/integration/068_partial_parsing_tests/test_pp_vars.py b/test/integration/068_partial_parsing_tests/test_pp_vars.py index e9e4779cf5d..ae0b2f066a9 100644 --- a/test/integration/068_partial_parsing_tests/test_pp_vars.py +++ b/test/integration/068_partial_parsing_tests/test_pp_vars.py @@ -268,7 +268,7 @@ def test_postgres_project_env_vars(self): model_id = 'model.test.model_one' model = manifest.nodes[model_id] self.assertEqual(model.config.meta['meta_name'], 'Jane Smith') - env_vars_hash_checksum = state_check.env_vars_hash.checksum + env_vars_hash_checksum = state_check.project_env_vars_hash.checksum # Change the environment variable os.environ['ENV_VAR_NAME'] = "Jane Doe" @@ -277,7 +277,60 @@ def test_postgres_project_env_vars(self): manifest = get_manifest() model = manifest.nodes[model_id] self.assertEqual(model.config.meta['meta_name'], 'Jane Doe') - self.assertNotEqual(env_vars_hash_checksum, manifest.state_check.env_vars_hash.checksum) + self.assertNotEqual(env_vars_hash_checksum, manifest.state_check.project_env_vars_hash.checksum) # cleanup del os.environ['ENV_VAR_NAME'] + +class ProfileEnvVarTest(BasePPTest): + + @property + def profile_config(self): + # Need to set these here because the base integration test class + # calls 'load_config' before the tests are run. + # Note: only the specified profile is rendered, so there's no + # point it setting env_vars in non-used profiles. + os.environ['ENV_VAR_USER'] = 'root' + os.environ['ENV_VAR_PASS'] = 'password' + return { + 'config': { + 'send_anonymous_usage_stats': False + }, + 'test': { + 'outputs': { + 'dev': { + 'type': 'postgres', + 'threads': 1, + 'host': self.database_host, + 'port': 5432, + 'user': "root", + 'pass': "password", + 'user': "{{ env_var('ENV_VAR_USER') }}", + 'pass': "{{ env_var('ENV_VAR_PASS') }}", + 'dbname': 'dbt', + 'schema': self.unique_schema() + }, + }, + 'target': 'dev' + } + } + + @use_profile('postgres') + def test_postgres_profile_env_vars(self): + + # Initial run + os.environ['ENV_VAR_USER'] = 'root' + os.environ['ENV_VAR_PASS'] = 'password' + self.setup_directories() + self.copy_file('test-files/model_one.sql', 'models/model_one.sql') + results = self.run_dbt(["run"]) + manifest = get_manifest() + env_vars_checksum = manifest.state_check.profile_env_vars_hash.checksum + + # Change env_vars + os.environ['ENV_VAR_PASS'] = 'my_pass' + (results, log_output) = self.run_dbt_and_capture(["run"], expect_pass=False) + self.assertTrue('env vars used in profiles.yml have changed' in log_output) + manifest = get_manifest() + self.assertNotEqual(env_vars_checksum, manifest.state_check.profile_env_vars_hash.checksum) + diff --git a/test/unit/test_config.py b/test/unit/test_config.py index c9e509fa622..48c713680e1 100644 --- a/test/unit/test_config.py +++ b/test/unit/test_config.py @@ -43,7 +43,7 @@ def raises_nothing(): def empty_profile_renderer(): - return dbt.config.renderer.ProfileRenderer(generate_base_context({})) + return dbt.config.renderer.ProfileRenderer({}) def empty_project_renderer(): @@ -509,7 +509,7 @@ def test_invalid_env_vars(self): def test_cli_and_env_vars(self): self.args.target = 'cli-and-env-vars' self.args.vars = '{"cli_value_host": "cli-postgres-host"}' - renderer = dbt.config.renderer.ProfileRenderer(generate_base_context({'cli_value_host': 'cli-postgres-host'})) + renderer = dbt.config.renderer.ProfileRenderer({'cli_value_host': 'cli-postgres-host'}) with mock.patch.dict(os.environ, self.env_override): profile = self.from_args(renderer=renderer) from_raw = self.from_raw_profile_info( diff --git a/test/unit/test_graph.py b/test/unit/test_graph.py index 7a26a9cbbeb..58f8b24fc4e 100644 --- a/test/unit/test_graph.py +++ b/test/unit/test_graph.py @@ -102,7 +102,8 @@ def _mock_state_check(self): config = self.root_project all_projects = self.all_projects return ManifestStateCheck( - env_vars_hash=FileHash.from_contents(''), + project_env_vars_hash=FileHash.from_contents(''), + profile_env_vars_hash=FileHash.from_contents(''), vars_hash=FileHash.from_contents('vars'), project_hashes={name: FileHash.from_contents(name) for name in all_projects}, profile_hash=FileHash.from_contents('profile'), diff --git a/test/unit/utils.py b/test/unit/utils.py index 3b3ed324507..9fa1f5f1582 100644 --- a/test/unit/utils.py +++ b/test/unit/utils.py @@ -47,7 +47,7 @@ def profile_from_dict(profile, profile_name, cli_vars='{}'): if not isinstance(cli_vars, dict): cli_vars = parse_cli_vars(cli_vars) - renderer = ProfileRenderer(generate_base_context(cli_vars)) + renderer = ProfileRenderer(cli_vars) return Profile.from_raw_profile_info( profile, profile_name,