From 42e8c56b133be901cd22d67ba94d678bf2b06970 Mon Sep 17 00:00:00 2001 From: Jacob Beck Date: Wed, 1 Apr 2020 15:36:29 -0600 Subject: [PATCH] in "dbt deps", if a profile cannot be found/loaded, use invalid values instead Add tests --- CHANGELOG.md | 3 +- core/dbt/config/__init__.py | 2 +- core/dbt/config/profile.py | 14 ++ core/dbt/config/runtime.py | 185 +++++++++++++++++- core/dbt/context/target.py | 14 +- core/dbt/contracts/connection.py | 3 + core/dbt/task/base.py | 4 +- core/dbt/task/deps.py | 10 +- .../test_simple_dependency.py | 21 +- 9 files changed, 225 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dde157e5d7a..d192bddb35f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,8 @@ - Fix an issue with raw blocks where multiple raw blocks in the same file resulted in an error ([#2241](https://github.com/fishtown-analytics/dbt/issues/2241), [#2252](https://github.com/fishtown-analytics/dbt/pull/2252)) - Fix a redshift-only issue that caused an error when `dbt seed` found a seed with an entirely empty column that was set to a `varchar` data type. ([#2250](https://github.com/fishtown-analytics/dbt/issues/2250), [#2254](https://github.com/fishtown-analytics/dbt/pull/2254)) - Fix a bug where third party plugins that used the default `list_schemas` and `information_schema_name` macros with database quoting enabled double-quoted the database name in their queries ([#2267](https://github.com/fishtown-analytics/dbt/issues/2267), [#2281](https://github.com/fishtown-analytics/dbt/pull/2281)) -- The BigQuery "partitions" config value can now be used in `dbt_project.yml` ([##2256](https://github.com/fishtown-analytics/dbt/issues/#2256), [#2280](https://github.com/fishtown-analytics/dbt/pull/2280)) +- The BigQuery "partitions" config value can now be used in `dbt_project.yml` ([#2256](https://github.com/fishtown-analytics/dbt/issues/2256), [#2280](https://github.com/fishtown-analytics/dbt/pull/2280)) +- dbt deps once again does not require a profile, but if profile-specific fields are accessed users will get an error ([#2231](https://github.com/fishtown-analytics/dbt/issues/2231), [#2290](https://github.com/fishtown-analytics/dbt/pull/2290)) ### Under the hood - Pin google libraries to higher minimum values, add more dependencies as explicit ([#2233](https://github.com/fishtown-analytics/dbt/issues/2233), [#2249](https://github.com/fishtown-analytics/dbt/pull/2249)) diff --git a/core/dbt/config/__init__.py b/core/dbt/config/__init__.py index d18fd7f0790..cb9a4481731 100644 --- a/core/dbt/config/__init__.py +++ b/core/dbt/config/__init__.py @@ -1,5 +1,5 @@ # all these are just exports, they need "noqa" so flake8 will not complain. from .profile import Profile, PROFILES_DIR, read_user_config # noqa from .project import Project # noqa -from .runtime import RuntimeConfig # noqa +from .runtime import RuntimeConfig, PoisonedProfileConfig # noqa from .renderer import ConfigRenderer # noqa diff --git a/core/dbt/config/profile.py b/core/dbt/config/profile.py index 618aecd8054..0b379aebba0 100644 --- a/core/dbt/config/profile.py +++ b/core/dbt/config/profile.py @@ -103,6 +103,20 @@ def to_profile_info( result['credentials'] = self.credentials.to_dict() return result + def to_target_dict(self) -> Dict[str, Any]: + target = dict( + self.credentials.connection_info(with_aliases=True) + ) + target.update({ + 'type': self.credentials.type, + 'threads': self.threads, + 'name': self.target_name, + 'target_name': self.target_name, + 'profile_name': self.profile_name, + 'config': self.config.to_dict(), + }) + return target + def __eq__(self, other: object) -> bool: if not (isinstance(other, self.__class__) and isinstance(self, other.__class__)): diff --git a/core/dbt/config/runtime.py b/core/dbt/config/runtime.py index 19a31b4a995..5a42f89a3a2 100644 --- a/core/dbt/config/runtime.py +++ b/core/dbt/config/runtime.py @@ -1,20 +1,22 @@ from copy import deepcopy -from dataclasses import dataclass +from dataclasses import dataclass, fields import os -from typing import Dict, Any +from typing import Dict, Any, Type from .profile import Profile from .project import Project from .renderer import ConfigRenderer -from dbt.utils import parse_cli_vars +from dbt import tracking +from dbt.adapters.factory import get_relation_class_by_name from dbt.context.base import generate_base_context from dbt.context.target import generate_target_context -from dbt.contracts.connection import AdapterRequiredConfig +from dbt.contracts.connection import AdapterRequiredConfig, Credentials from dbt.contracts.graph.manifest import ManifestMetadata -from dbt.contracts.project import Configuration -from dbt.exceptions import DbtProjectError +from dbt.contracts.project import Configuration, UserConfig +from dbt.logger import GLOBAL_LOGGER as logger +from dbt.exceptions import DbtProjectError, RuntimeException, DbtProfileError from dbt.exceptions import validator_error_message -from dbt.adapters.factory import get_relation_class_by_name +from dbt.utils import parse_cli_vars from hologram import ValidationError @@ -171,3 +173,172 @@ def get_metadata(self) -> ManifestMetadata: project_id=self.hashed_name(), adapter_type=self.credentials.type ) + + +class PoisonedCredentials(Credentials): + def __init__(self): + super().__init__('', '') + + @property + def type(self): + return None + + def connection_info(self, *args, **kwargs): + return {} + + def _connection_keys(self): + return () + + +class PoisonedConfig(UserConfig): + def __getattribute__(self, name): + if name in {f.name for f in fields(UserConfig)}: + raise AttributeError( + f"'PoisonedConfig' object has no attribute {name}" + ) + + def to_dict(self): + return {} + + +class PoisonedProfile(Profile): + def __init__(self): + self.credentials = PoisonedCredentials() + self.config = PoisonedConfig() + self.profile_name = '' + self.target_name = '' + self.threads = -1 + + def to_target_dict(self): + return {} + + def __getattribute__(self, name): + if name in {'profile_name', 'target_name', 'threads'}: + raise RuntimeException( + f'Error: disallowed attribute "{name}" - no profile!' + ) + + return Profile.__getattribute__(self, name) + + +@dataclass +class PoisonedProfileConfig(RuntimeConfig): + """This class acts a lot _like_ a RuntimeConfig, except if your profile is + missing, any access to profile members results in an exception. + """ + + def __post_init__(self): + # instead of futzing with InitVar overrides or rewriting __init__, just + # `del` the attrs we don't want users touching. + del self.profile_name + del self.target_name + # don't call super().__post_init__(), as that calls validate(), and + # this object isn't very valid + + def __getattribute__(self, name): + # Override __getattribute__ to check that the attribute isn't 'banned'. + if name in {'profile_name', 'target_name'}: + raise RuntimeException( + f'Error: disallowed attribute "{name}" - no profile!' + ) + + # avoid every attribute access triggering infinite recursion + return RuntimeConfig.__getattribute__(self, name) + + def to_target_dict(self): + # re-override the poisoned profile behavior + return {} + + @classmethod + def from_parts( + cls, project: Project, profile: Any, args: Any, + ) -> 'RuntimeConfig': + """Instantiate a RuntimeConfig from its components. + + :param profile: Ignored. + :param project: A parsed dbt Project. + :param args: The parsed command-line arguments. + :returns RuntimeConfig: The new configuration. + """ + cli_vars: Dict[str, Any] = parse_cli_vars(getattr(args, 'vars', '{}')) + + return cls( + project_name=project.project_name, + version=project.version, + project_root=project.project_root, + source_paths=project.source_paths, + macro_paths=project.macro_paths, + data_paths=project.data_paths, + test_paths=project.test_paths, + analysis_paths=project.analysis_paths, + docs_paths=project.docs_paths, + target_path=project.target_path, + snapshot_paths=project.snapshot_paths, + clean_targets=project.clean_targets, + log_path=project.log_path, + modules_path=project.modules_path, + quoting=project.quoting, # we never use this anyway. + models=project.models, + on_run_start=project.on_run_start, + on_run_end=project.on_run_end, + seeds=project.seeds, + snapshots=project.snapshots, + dbt_version=project.dbt_version, + packages=project.packages, + query_comment=project.query_comment, + profile_name='', + target_name='', + config=PoisonedConfig(), + threads=getattr(args, 'threads', 1), + credentials=PoisonedCredentials(), + args=args, + cli_vars=cli_vars, + ) + + @classmethod + def from_args(cls: Type[RuntimeConfig], args: Any) -> 'RuntimeConfig': + """Given arguments, read in dbt_project.yml from the current directory, + read in packages.yml if it exists, and use them to find the profile to + load. + + :param args: The arguments as parsed from the cli. + :raises DbtProjectError: If the project is invalid or missing. + :raises DbtProfileError: If the profile is invalid or missing. + :raises ValidationException: If the cli variables are invalid. + """ + # profile_name from the project + partial = Project.partial_load(os.getcwd()) + + # build the profile using the base renderer and the one fact we know + cli_vars: Dict[str, Any] = parse_cli_vars(getattr(args, 'vars', '{}')) + renderer = ConfigRenderer(generate_base_context(cli_vars=cli_vars)) + profile_name = partial.render_profile_name(renderer) + + try: + profile = Profile.render_from_args( + args, renderer, profile_name + ) + cls = RuntimeConfig # we can return a real runtime config, do that + except (DbtProjectError, DbtProfileError) as exc: + logger.debug( + 'Profile not loaded due to error: {}', exc, exc_info=True + ) + logger.info( + 'No profile "{}" found, continuing with no target', + profile_name + ) + # return the poisoned form + profile = PoisonedProfile() + # disable anonymous usage statistics + tracking.do_not_track() + + # get a new renderer using our target information and render the + # project + renderer = ConfigRenderer(generate_target_context(profile, cli_vars)) + project = partial.render(renderer) + + return cls.from_parts( + project=project, + profile=profile, + args=args, + ) diff --git a/core/dbt/context/target.py b/core/dbt/context/target.py index 489ac42ab26..93cc2a461e2 100644 --- a/core/dbt/context/target.py +++ b/core/dbt/context/target.py @@ -14,19 +14,7 @@ def __init__(self, config: HasCredentials, cli_vars: Dict[str, Any]): @contextproperty def target(self) -> Dict[str, Any]: - target = dict( - self.config.credentials.connection_info(with_aliases=True) - ) - target.update({ - 'type': self.config.credentials.type, - 'threads': self.config.threads, - 'name': self.config.target_name, - # not specified, but present for compatibility - 'target_name': self.config.target_name, - 'profile_name': self.config.profile_name, - 'config': self.config.config.to_dict(), - }) - return target + return self.config.to_target_dict() def generate_target_context( diff --git a/core/dbt/contracts/connection.py b/core/dbt/contracts/connection.py index 47ecdaa26e3..e786cf5f748 100644 --- a/core/dbt/contracts/connection.py +++ b/core/dbt/contracts/connection.py @@ -171,6 +171,9 @@ class HasCredentials(Protocol): target_name: str threads: int + def to_target_dict(self): + raise NotImplementedError('to_target_dict not implemented') + DEFAULT_QUERY_COMMENT = ''' {%- set comment_dict = {} -%} diff --git a/core/dbt/task/base.py b/core/dbt/task/base.py index 2a113c6b831..a872115dc4a 100644 --- a/core/dbt/task/base.py +++ b/core/dbt/task/base.py @@ -64,7 +64,7 @@ def from_args(cls, args): tracking.track_invalid_invocation( args=args, result_type=exc.result_type) - raise dbt.exceptions.RuntimeException('Could not run dbt') + raise dbt.exceptions.RuntimeException('Could not run dbt') from exc except dbt.exceptions.DbtProfileError as exc: logger.error("Encountered an error while reading profiles:") logger.error(" ERROR {}".format(str(exc))) @@ -84,7 +84,7 @@ def from_args(cls, args): tracking.track_invalid_invocation( args=args, result_type=exc.result_type) - raise dbt.exceptions.RuntimeException('Could not run dbt') + raise dbt.exceptions.RuntimeException('Could not run dbt') from exc return cls(args, config) @abstractmethod diff --git a/core/dbt/task/deps.py b/core/dbt/task/deps.py index 68dbf0e6976..78b13b24a94 100644 --- a/core/dbt/task/deps.py +++ b/core/dbt/task/deps.py @@ -2,7 +2,7 @@ import dbt.deprecations import dbt.exceptions -from dbt.config import RuntimeConfig, ConfigRenderer +from dbt.config import PoisonedProfileConfig, ConfigRenderer from dbt.context.target import generate_target_context from dbt.deps.base import downloads_directory from dbt.deps.resolver import resolve_packages @@ -10,11 +10,13 @@ from dbt.logger import GLOBAL_LOGGER as logger from dbt.clients import system -from dbt.task.base import ConfiguredTask +from dbt.task.base import BaseTask -class DepsTask(ConfiguredTask): - def __init__(self, args, config: RuntimeConfig): +class DepsTask(BaseTask): + ConfigType = PoisonedProfileConfig + + def __init__(self, args, config: PoisonedProfileConfig): super().__init__(args=args, config=config) def track_package_install( diff --git a/test/integration/006_simple_dependency_test/test_simple_dependency.py b/test/integration/006_simple_dependency_test/test_simple_dependency.py index 665a4c3d51b..a7e64821d28 100644 --- a/test/integration/006_simple_dependency_test/test_simple_dependency.py +++ b/test/integration/006_simple_dependency_test/test_simple_dependency.py @@ -1,4 +1,6 @@ import os +import shutil +import tempfile from test.integration.base import DBTIntegrationTest, use_profile from dbt.exceptions import CompilationException @@ -28,9 +30,12 @@ def packages_config(self): ] } + def run_deps(self): + return self.run_dbt(["deps"]) + @use_profile('postgres') def test_postgres_simple_dependency(self): - self.run_dbt(["deps"]) + self.run_deps() results = self.run_dbt(["run"]) self.assertEqual(len(results), 4) @@ -42,7 +47,7 @@ def test_postgres_simple_dependency(self): self.run_sql_file("update.sql") - self.run_dbt(["deps"]) + self.run_deps() results = self.run_dbt(["run"]) self.assertEqual(len(results), 4) @@ -52,7 +57,7 @@ def test_postgres_simple_dependency(self): @use_profile('postgres') def test_postgres_simple_dependency_with_models(self): - self.run_dbt(["deps"]) + self.run_deps() results = self.run_dbt(["run", '--models', 'view_model+']) self.assertEqual(len(results), 2) @@ -220,3 +225,13 @@ def test_postgres_empty_models_not_compiled_in_dependencies(self): models = self.get_models_in_schema() self.assertFalse('empty' in models.keys()) + + +class TestSimpleDependencyNoProfile(TestSimpleDependency): + def run_deps(self): + tmpdir = tempfile.mkdtemp() + try: + result = self.run_dbt(["deps", "--profiles-dir", tmpdir]) + finally: + shutil.rmtree(tmpdir) + return result