From 28776a21e6c17f95218482d001abebc5a35842c9 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Wed, 31 May 2023 01:30:52 -0700 Subject: [PATCH 01/16] working on it --- core/dbt/adapters/factory.py | 3 ++ core/dbt/cli/main.py | 3 +- core/dbt/cli/params.py | 7 +++++ core/dbt/task/debug.py | 57 ++++++++++++++++++++++++++++++++---- core/dbt/version.py | 1 + 5 files changed, 65 insertions(+), 6 deletions(-) diff --git a/core/dbt/adapters/factory.py b/core/dbt/adapters/factory.py index fffde6b487f..53a54c73d1a 100644 --- a/core/dbt/adapters/factory.py +++ b/core/dbt/adapters/factory.py @@ -165,6 +165,9 @@ def get_adapter_constraint_support(self, name: Optional[str]) -> List[str]: FACTORY: AdapterContainer = AdapterContainer() +def get_adapter_plugins(name: Optional[str]) -> List[AdapterPlugin]: + return FACTORY.get_adapter_plugins(name) + def register_adapter(config: AdapterRequiredConfig) -> None: FACTORY.register_adapter(config) diff --git a/core/dbt/cli/main.py b/core/dbt/cli/main.py index d599a7a4d5e..15784d5072e 100644 --- a/core/dbt/cli/main.py +++ b/core/dbt/cli/main.py @@ -404,12 +404,13 @@ def show(ctx, **kwargs): @p.profiles_dir_exists_false @p.project_dir @p.target +@p.test_connection_only @p.vars @p.version_check @requires.postflight @requires.preflight def debug(ctx, **kwargs): - """Test the database connection and show information for debugging purposes. Not to be confused with the --debug option which increases verbosity.""" + """Show information on the current dbt environment and check dependencies, then test the database connection. Not to be confused with the --debug option which increases verbosity.""" task = DebugTask( ctx.obj["flags"], diff --git a/core/dbt/cli/params.py b/core/dbt/cli/params.py index b638fc539dc..48b102fd134 100644 --- a/core/dbt/cli/params.py +++ b/core/dbt/cli/params.py @@ -478,6 +478,13 @@ type=click.Path(), ) +test_connection_only = click.option( + "--test-connection-only", + envvar=None, + help="Test the connection to the database, then exit, without dependency checks.", + is_flag=True, +) + threads = click.option( "--threads", envvar=None, diff --git a/core/dbt/task/debug.py b/core/dbt/task/debug.py index 8e70cb68da6..4e930e9cd50 100644 --- a/core/dbt/task/debug.py +++ b/core/dbt/task/debug.py @@ -1,7 +1,9 @@ # coding=utf-8 +import re import os import platform import sys +from pathlib import Path from typing import Optional, Dict, Any, List from dbt.events.functions import fire_event @@ -12,7 +14,7 @@ ) import dbt.clients.system import dbt.exceptions -from dbt.adapters.factory import get_adapter, register_adapter +from dbt.adapters.factory import get_adapter, register_adapter, get_adapter_plugins from dbt.config import PartialProject, Project, Profile from dbt.config.renderer import DbtProjectYamlRenderer, ProfileRenderer from dbt.clients.yaml_helper import load_yaml_text @@ -101,14 +103,30 @@ def run(self): return not self.any_failure version = get_installed_version().to_version_string(skip_matcher=True) + + # Collect metadata needed to log debug information + profile_status = self._load_profile() + adapter = self._get_adapter(self.profile) + adapter_type, adapter_version = self._read_adapter_info(adapter.__class__) + fire_event(DebugCmdOut(msg="dbt version: {}".format(version))) + fire_event(DebugCmdOut(msg="adapter type: {}".format(adapter_type))) + fire_event(DebugCmdOut(msg="adapter version: {}".format(adapter_version))) fire_event(DebugCmdOut(msg="python version: {}".format(sys.version.split()[0]))) fire_event(DebugCmdOut(msg="python path: {}".format(sys.executable))) fire_event(DebugCmdOut(msg="os info: {}".format(platform.platform()))) fire_event(DebugCmdOut(msg="Using profiles.yml file at {}".format(self.profile_path))) fire_event(DebugCmdOut(msg="Using dbt_project.yml file at {}".format(self.project_path))) - self.test_configuration() - self.test_dependencies() + + # some users want to test connection without verifying upstream dependencies + breakpoint() + if self.args.test_connection_only: + fire_event(DebugCmdOut(msg="Skipping steps before connection verification")) + project_status = self._load_project() + else: + self.test_configuration() + self.test_dependencies() + self.test_connection() if self.any_failure: @@ -126,6 +144,35 @@ def run(self): def interpret_results(self, results): return results + def _get_adapter(self, profile): + '''Using profile, return the adapter instance.''' + register_adapter(profile) + return get_adapter(profile) + + def _read_adapter_info(self, cls): + '''Read the adapter name and version from the setup.py file of the adapter plugin.''' + current_adapter_plugin = next(filter(lambda x: x.adapter == cls, get_adapter_plugins(None))) + adapter_setup_script = current_adapter_plugin.include_path / Path("../../../setup.py") + + try: + with open(adapter_setup_script.resolve()) as f: + adapter_setup_code = f.read() + except FileNotFoundError: + adapter_name = red("ERROR not found") + version_str = red("ERROR not found") + else: + pattern = r'package_name = "(.*)"\n' + match = re.search(pattern, adapter_setup_code) + adapter_name = match.group(1) if match else red("ERROR not found") + + pattern = r'package_version = "(.*)"\n' + match = re.search(pattern, adapter_setup_code) + version_str = match.group(1) if match else red("ERROR not found") + + return adapter_name, version_str + + + def _load_project(self): if not os.path.exists(self.project_path): self.project_fail_details = FILE_NOT_FOUND @@ -291,8 +338,8 @@ def test_dependencies(self): def test_configuration(self): fire_event(DebugCmdOut(msg="Configuration:")) - profile_status = self._load_profile() - fire_event(DebugCmdOut(msg=f" profiles.yml file [{profile_status}]")) + # profile_status = self._load_profile() + # fire_event(DebugCmdOut(msg=f" profiles.yml file [{profile_status}]")) project_status = self._load_project() fire_event(DebugCmdOut(msg=f" dbt_project.yml file [{project_status}]")) diff --git a/core/dbt/version.py b/core/dbt/version.py index d10c10e7186..ae4da6f202c 100644 --- a/core/dbt/version.py +++ b/core/dbt/version.py @@ -216,6 +216,7 @@ def _get_dbt_plugins_info() -> Iterator[Tuple[str, str]]: def _get_adapter_plugin_names() -> Iterator[str]: + breakpoint() spec = importlib.util.find_spec("dbt.adapters") # If None, then nothing provides an importable 'dbt.adapters', so we will # not be reporting plugin versions today From b4ea0230e2cbb89cf9c62f89f251c1587fb4ff20 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Wed, 31 May 2023 01:50:11 -0700 Subject: [PATCH 02/16] Commit the method add --- core/dbt/adapters/factory.py | 1 + core/dbt/task/debug.py | 26 ++++++++++++-------------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/core/dbt/adapters/factory.py b/core/dbt/adapters/factory.py index 53a54c73d1a..986bab664de 100644 --- a/core/dbt/adapters/factory.py +++ b/core/dbt/adapters/factory.py @@ -168,6 +168,7 @@ def get_adapter_constraint_support(self, name: Optional[str]) -> List[str]: def get_adapter_plugins(name: Optional[str]) -> List[AdapterPlugin]: return FACTORY.get_adapter_plugins(name) + def register_adapter(config: AdapterRequiredConfig) -> None: FACTORY.register_adapter(config) diff --git a/core/dbt/task/debug.py b/core/dbt/task/debug.py index 4e930e9cd50..1960c4ecaf2 100644 --- a/core/dbt/task/debug.py +++ b/core/dbt/task/debug.py @@ -118,15 +118,15 @@ def run(self): fire_event(DebugCmdOut(msg="Using profiles.yml file at {}".format(self.profile_path))) fire_event(DebugCmdOut(msg="Using dbt_project.yml file at {}".format(self.project_path))) - # some users want to test connection without verifying upstream dependencies - breakpoint() + # Skip upstream dependency checks for users who want to test their connection only if self.args.test_connection_only: fire_event(DebugCmdOut(msg="Skipping steps before connection verification")) - project_status = self._load_project() + self._load_project() # would be called in test_configuration else: - self.test_configuration() + self.test_configuration(profile_status) self.test_dependencies() + # Test connection self.test_connection() if self.any_failure: @@ -145,13 +145,15 @@ def interpret_results(self, results): return results def _get_adapter(self, profile): - '''Using profile, return the adapter instance.''' + """Using profile, return the adapter instance.""" register_adapter(profile) return get_adapter(profile) def _read_adapter_info(self, cls): - '''Read the adapter name and version from the setup.py file of the adapter plugin.''' - current_adapter_plugin = next(filter(lambda x: x.adapter == cls, get_adapter_plugins(None))) + """Read the adapter name and version from the setup.py file of the adapter plugin.""" + current_adapter_plugin = next( + filter(lambda x: x.adapter == cls, get_adapter_plugins(None)) + ) adapter_setup_script = current_adapter_plugin.include_path / Path("../../../setup.py") try: @@ -171,8 +173,6 @@ def _read_adapter_info(self, cls): return adapter_name, version_str - - def _load_project(self): if not os.path.exists(self.project_path): self.project_fail_details = FILE_NOT_FOUND @@ -335,12 +335,9 @@ def test_dependencies(self): logline_msg = self.test_git() fire_event(DebugCmdResult(msg=f" - git [{logline_msg}]\n")) - def test_configuration(self): + def test_configuration(self, profile_status): fire_event(DebugCmdOut(msg="Configuration:")) - - # profile_status = self._load_profile() - # fire_event(DebugCmdOut(msg=f" profiles.yml file [{profile_status}]")) - + fire_event(DebugCmdOut(msg=f" profiles.yml file [{profile_status}]")) project_status = self._load_project() fire_event(DebugCmdOut(msg=f" dbt_project.yml file [{project_status}]")) @@ -397,6 +394,7 @@ def attempt_connection(profile): adapter = get_adapter(profile) try: with adapter.connection_named("debug"): + breakpoint() adapter.debug_query() except Exception as exc: return COULD_NOT_CONNECT_MESSAGE.format( From f7cc0edfcdd548411a1f16ebf10857a8c4b1ea3e Mon Sep 17 00:00:00 2001 From: Mila Page Date: Wed, 31 May 2023 02:07:51 -0700 Subject: [PATCH 03/16] remove the breakpoints --- core/dbt/task/debug.py | 1 - core/dbt/version.py | 1 - 2 files changed, 2 deletions(-) diff --git a/core/dbt/task/debug.py b/core/dbt/task/debug.py index 1960c4ecaf2..8cd6ca50a27 100644 --- a/core/dbt/task/debug.py +++ b/core/dbt/task/debug.py @@ -394,7 +394,6 @@ def attempt_connection(profile): adapter = get_adapter(profile) try: with adapter.connection_named("debug"): - breakpoint() adapter.debug_query() except Exception as exc: return COULD_NOT_CONNECT_MESSAGE.format( diff --git a/core/dbt/version.py b/core/dbt/version.py index ae4da6f202c..d10c10e7186 100644 --- a/core/dbt/version.py +++ b/core/dbt/version.py @@ -216,7 +216,6 @@ def _get_dbt_plugins_info() -> Iterator[Tuple[str, str]]: def _get_adapter_plugin_names() -> Iterator[str]: - breakpoint() spec = importlib.util.find_spec("dbt.adapters") # If None, then nothing provides an importable 'dbt.adapters', so we will # not be reporting plugin versions today From 24e2d476fd5d0435f087f84a3cfcc8a07f18034d Mon Sep 17 00:00:00 2001 From: Mila Page Date: Sat, 3 Jun 2023 16:17:50 -0700 Subject: [PATCH 04/16] Remove open command rather than fixing issue #7774 --- core/dbt/cli/main.py | 1 - core/dbt/cli/params.py | 7 ------- core/dbt/clients/system.py | 12 ------------ core/dbt/events/types.proto | 11 ----------- core/dbt/events/types.py | 12 ------------ core/dbt/task/debug.py | 10 +--------- tests/unit/test_events.py | 1 - 7 files changed, 1 insertion(+), 53 deletions(-) diff --git a/core/dbt/cli/main.py b/core/dbt/cli/main.py index 15784d5072e..91788e3325c 100644 --- a/core/dbt/cli/main.py +++ b/core/dbt/cli/main.py @@ -399,7 +399,6 @@ def show(ctx, **kwargs): # dbt debug @cli.command("debug") @click.pass_context -@p.config_dir @p.profile @p.profiles_dir_exists_false @p.project_dir diff --git a/core/dbt/cli/params.py b/core/dbt/cli/params.py index 48b102fd134..e39d61b3284 100644 --- a/core/dbt/cli/params.py +++ b/core/dbt/cli/params.py @@ -40,13 +40,6 @@ default=True, ) -config_dir = click.option( - "--config-dir", - envvar=None, - help="Show the configured location for the profiles.yml file and exit", - is_flag=True, -) - debug = click.option( "--debug/--no-debug", "-d/ ", diff --git a/core/dbt/clients/system.py b/core/dbt/clients/system.py index 66c59354b4f..31b1de2cbeb 100644 --- a/core/dbt/clients/system.py +++ b/core/dbt/clients/system.py @@ -323,18 +323,6 @@ def path_is_symlink(path: str) -> bool: return os.path.islink(path) -def open_dir_cmd() -> str: - # https://docs.python.org/2/library/sys.html#sys.platform - if sys.platform == "win32": - return "start" - - elif sys.platform == "darwin": - return "open" - - else: - return "xdg-open" - - def _handle_posix_cwd_error(exc: OSError, cwd: str, cmd: List[str]) -> NoReturn: if exc.errno == errno.ENOENT: message = "Directory does not exist" diff --git a/core/dbt/events/types.proto b/core/dbt/events/types.proto index a8c3bee405a..c1f80cf97c2 100644 --- a/core/dbt/events/types.proto +++ b/core/dbt/events/types.proto @@ -2045,17 +2045,6 @@ message FinishedCleanPathsMsg { FinishedCleanPaths data = 2; } -// Z016 -message OpenCommand { - string open_cmd = 1; - string profiles_dir = 2; -} - -message OpenCommandMsg { - EventInfo info = 1; - OpenCommand data = 2; -} - // Z017 message Formatting { string msg = 1; diff --git a/core/dbt/events/types.py b/core/dbt/events/types.py index b5abc73aabe..d9bfac87918 100644 --- a/core/dbt/events/types.py +++ b/core/dbt/events/types.py @@ -1995,18 +1995,6 @@ def message(self) -> str: return "Finished cleaning all paths." -class OpenCommand(InfoLevel): - def code(self): - return "Z016" - - def message(self) -> str: - msg = f"""To view your profiles.yml file, run: - -{self.open_cmd} {self.profiles_dir}""" - - return msg - - # We use events to create console output, but also think of them as a sequence of important and # meaningful occurrences to be used for debugging and monitoring. The Formatting event helps eases # the tension between these two goals by allowing empty lines, heading separators, and other diff --git a/core/dbt/task/debug.py b/core/dbt/task/debug.py index 8cd6ca50a27..567f1c0d3a5 100644 --- a/core/dbt/task/debug.py +++ b/core/dbt/task/debug.py @@ -8,7 +8,6 @@ from dbt.events.functions import fire_event from dbt.events.types import ( - OpenCommand, DebugCmdOut, DebugCmdResult, ) @@ -93,15 +92,7 @@ def project_profile(self): return None return self.project.profile_name - def path_info(self): - open_cmd = dbt.clients.system.open_dir_cmd() - fire_event(OpenCommand(open_cmd=open_cmd, profiles_dir=self.profiles_dir)) - def run(self): - if self.args.config_dir: - self.path_info() - return not self.any_failure - version = get_installed_version().to_version_string(skip_matcher=True) # Collect metadata needed to log debug information @@ -115,6 +106,7 @@ def run(self): fire_event(DebugCmdOut(msg="python version: {}".format(sys.version.split()[0]))) fire_event(DebugCmdOut(msg="python path: {}".format(sys.executable))) fire_event(DebugCmdOut(msg="os info: {}".format(platform.platform()))) + fire_event(DebugCmdOut(msg="Using profiles dir at {}".format(self.profiles_dir))) fire_event(DebugCmdOut(msg="Using profiles.yml file at {}".format(self.profile_path))) fire_event(DebugCmdOut(msg="Using dbt_project.yml file at {}".format(self.project_path))) diff --git a/tests/unit/test_events.py b/tests/unit/test_events.py index d921ee87d5f..e94274871f3 100644 --- a/tests/unit/test_events.py +++ b/tests/unit/test_events.py @@ -364,7 +364,6 @@ def test_event_codes(self): types.ConfirmCleanPath(path=""), types.ProtectedCleanPath(path=""), types.FinishedCleanPaths(), - types.OpenCommand(open_cmd="", profiles_dir=""), types.RunResultWarning(resource_type="", node_name="", path=""), types.RunResultFailure(resource_type="", node_name="", path=""), types.StatsLine(stats={"error": 0, "skip": 0, "pass": 0, "warn": 0, "total": 0}), From b5c1c9f66bd700eee90b3e11d90c94f0f614cddd Mon Sep 17 00:00:00 2001 From: Mila Page Date: Sun, 4 Jun 2023 02:41:23 -0700 Subject: [PATCH 05/16] Adjust for mypy --- core/dbt/cli/main.py | 2 +- core/dbt/cli/params.py | 6 +- core/dbt/task/debug.py | 438 +++++++++++++++++++++++------------------ core/dbt/version.py | 2 + 4 files changed, 248 insertions(+), 200 deletions(-) diff --git a/core/dbt/cli/main.py b/core/dbt/cli/main.py index 49f67baecee..0a7608c1f70 100644 --- a/core/dbt/cli/main.py +++ b/core/dbt/cli/main.py @@ -401,11 +401,11 @@ def show(ctx, **kwargs): # dbt debug @cli.command("debug") @click.pass_context +@p.debug_connection @p.profile @p.profiles_dir_exists_false @p.project_dir @p.target -@p.test_connection_only @p.vars @p.version_check @requires.postflight diff --git a/core/dbt/cli/params.py b/core/dbt/cli/params.py index 373fa280152..9173f5f2af2 100644 --- a/core/dbt/cli/params.py +++ b/core/dbt/cli/params.py @@ -484,10 +484,10 @@ type=click.Path(), ) -test_connection_only = click.option( - "--test-connection-only", +debug_connection = click.option( + "--connection", envvar=None, - help="Test the connection to the database, then exit, without dependency checks.", + help="Test the connection to the target database independent of dependency checks.", is_flag=True, ) diff --git a/core/dbt/task/debug.py b/core/dbt/task/debug.py index 567f1c0d3a5..43f889dc25d 100644 --- a/core/dbt/task/debug.py +++ b/core/dbt/task/debug.py @@ -1,10 +1,12 @@ # coding=utf-8 -import re +import importlib import os import platform import sys -from pathlib import Path -from typing import Optional, Dict, Any, List + +from collections import namedtuple +from enum import Enum +from typing import Optional, Dict, Any, List, Tuple from dbt.events.functions import fire_event from dbt.events.types import ( @@ -13,7 +15,7 @@ ) import dbt.clients.system import dbt.exceptions -from dbt.adapters.factory import get_adapter, register_adapter, get_adapter_plugins +from dbt.adapters.factory import get_adapter, register_adapter from dbt.config import PartialProject, Project, Profile from dbt.config.renderer import DbtProjectYamlRenderer, ProfileRenderer from dbt.clients.yaml_helper import load_yaml_text @@ -58,6 +60,16 @@ FILE_NOT_FOUND = "file not found" +SubtaskStatus = namedtuple( + "SubtaskStatus", ["log_msg", "run_status", "details", "summary_message"] +) + + +class RunStatus(Enum): + PASS = 1 + FAIL = 2 + SKIP = 3 + class DebugTask(BaseTask): def __init__(self, args, config): @@ -83,7 +95,6 @@ def __init__(self, args, config): self.profile_name: Optional[str] = None self.project: Optional[Project] = None self.project_fail_details = "" - self.any_failure = False self.messages: List[str] = [] @property @@ -93,122 +104,121 @@ def project_profile(self): return self.project.profile_name def run(self): + # Basic info version = get_installed_version().to_version_string(skip_matcher=True) - - # Collect metadata needed to log debug information - profile_status = self._load_profile() - adapter = self._get_adapter(self.profile) - adapter_type, adapter_version = self._read_adapter_info(adapter.__class__) - fire_event(DebugCmdOut(msg="dbt version: {}".format(version))) - fire_event(DebugCmdOut(msg="adapter type: {}".format(adapter_type))) - fire_event(DebugCmdOut(msg="adapter version: {}".format(adapter_version))) fire_event(DebugCmdOut(msg="python version: {}".format(sys.version.split()[0]))) fire_event(DebugCmdOut(msg="python path: {}".format(sys.executable))) fire_event(DebugCmdOut(msg="os info: {}".format(platform.platform()))) + + # Load profile if possible, then load adapter info (which requires the profile) + load_profile_status = self._load_profile() fire_event(DebugCmdOut(msg="Using profiles dir at {}".format(self.profiles_dir))) fire_event(DebugCmdOut(msg="Using profiles.yml file at {}".format(self.profile_path))) fire_event(DebugCmdOut(msg="Using dbt_project.yml file at {}".format(self.project_path))) + if load_profile_status.run_status == RunStatus.PASS: + adapter_type = self.profile.credentials.type + adapter_version = self._read_adapter_version( + f"dbt.adapters.{adapter_type}.__version__" + ) + fire_event(DebugCmdOut(msg="adapter type: {}".format(adapter_type))) + fire_event(DebugCmdOut(msg="adapter version: {}".format(adapter_version))) - # Skip upstream dependency checks for users who want to test their connection only - if self.args.test_connection_only: + # Get project loaded to do additional checks + load_project_status = self._load_project() + if self.args.connection: fire_event(DebugCmdOut(msg="Skipping steps before connection verification")) - self._load_project() # would be called in test_configuration else: - self.test_configuration(profile_status) - self.test_dependencies() + # this job's status not logged since already accounted for in _load_* commands + self.test_configuration(load_profile_status.log_msg, load_project_status.log_msg) + dependencies_statuses = self.test_dependencies() # Test connection self.test_connection() - if self.any_failure: - fire_event( - DebugCmdResult(msg=red(f"{(pluralize(len(self.messages), 'check'))} failed:")) - ) + # Log messages from any fails + all_statuses = [load_profile_status, load_project_status, *dependencies_statuses] + failure_count = sum(1 for status in all_statuses if status.run_status == RunStatus.FAIL) + if failure_count > 0: + fire_event(DebugCmdResult(msg=red(f"{(pluralize(failure_count, 'check'))} failed:"))) else: fire_event(DebugCmdResult(msg=green("All checks passed!"))) - for message in self.messages: - fire_event(DebugCmdResult(msg=f"{message}\n")) + for status in filter(lambda status: status.run_status == RunStatus.FAIL, all_statuses): + fire_event(DebugCmdResult(msg=f"{status.summary_message}\n")) + + return failure_count == 0 - return not self.any_failure + # ============================== + # Override for elsewhere in core + # ============================== def interpret_results(self, results): return results - def _get_adapter(self, profile): - """Using profile, return the adapter instance.""" - register_adapter(profile) - return get_adapter(profile) - - def _read_adapter_info(self, cls): - """Read the adapter name and version from the setup.py file of the adapter plugin.""" - current_adapter_plugin = next( - filter(lambda x: x.adapter == cls, get_adapter_plugins(None)) - ) - adapter_setup_script = current_adapter_plugin.include_path / Path("../../../setup.py") - - try: - with open(adapter_setup_script.resolve()) as f: - adapter_setup_code = f.read() - except FileNotFoundError: - adapter_name = red("ERROR not found") - version_str = red("ERROR not found") - else: - pattern = r'package_name = "(.*)"\n' - match = re.search(pattern, adapter_setup_code) - adapter_name = match.group(1) if match else red("ERROR not found") + # =============== + # Loading profile + # =============== - pattern = r'package_version = "(.*)"\n' - match = re.search(pattern, adapter_setup_code) - version_str = match.group(1) if match else red("ERROR not found") - - return adapter_name, version_str + def _load_profile(self) -> SubtaskStatus: + """ + Side effect: load self.profile + Side effect: load self.target_name + """ + if not os.path.exists(self.profile_path): + return SubtaskStatus( + log_msg=red("ERROR not found"), + run_status=RunStatus.FAIL, + details=FILE_NOT_FOUND, + summary_message=MISSING_PROFILE_MESSAGE.format( + path=self.profile_path, url=ProfileConfigDocs + ), + ) - def _load_project(self): - if not os.path.exists(self.project_path): - self.project_fail_details = FILE_NOT_FOUND - return red("ERROR not found") + raw_profile_data = load_yaml_text(dbt.clients.system.load_file_contents(self.profile_path)) + if isinstance(raw_profile_data, dict): + self.raw_profile_data = raw_profile_data - renderer = DbtProjectYamlRenderer(self.profile, self.cli_vars) + profile_errors = [] + profile_names, summary_message = self._choose_profile_names() + renderer = ProfileRenderer(self.cli_vars) + for profile_name in profile_names: + try: + profile: Profile = Profile.render( + renderer, + profile_name, + self.args.profile, + self.args.target, + # TODO: Generalize safe access to flags.THREADS: + # https://github.com/dbt-labs/dbt-core/issues/6259 + getattr(self.args, "threads", None), + ) + except dbt.exceptions.DbtConfigError as exc: + profile_errors.append(str(exc)) + else: + if len(profile_names) == 1: + # if a profile was specified, set it on the task + self.target_name = self._choose_target_name(profile_name) + self.profile = profile - try: - self.project = Project.from_project_root( - self.project_dir, - renderer, - verify_version=self.args.VERSION_CHECK, + if profile_errors: + details = "\n\n".join(profile_errors) + return SubtaskStatus( + log_msg=red("ERROR invalid"), + run_status=RunStatus.FAIL, + details=details, + summary_message=summary_message + + (f"Profile loading failed for the following reason:" f"\n{details}" f"\n"), ) - except dbt.exceptions.DbtConfigError as exc: - self.project_fail_details = str(exc) - return red("ERROR invalid") - - return green("OK found and valid") - - def _profile_found(self): - if not self.raw_profile_data: - return red("ERROR not found") - assert self.raw_profile_data is not None - if self.profile_name in self.raw_profile_data: - return green("OK found") else: - return red("ERROR not found") - - def _target_found(self): - requirements = self.raw_profile_data and self.profile_name and self.target_name - if not requirements: - return red("ERROR not found") - # mypy appeasement, we checked just above - assert self.raw_profile_data is not None - assert self.profile_name is not None - assert self.target_name is not None - if self.profile_name not in self.raw_profile_data: - return red("ERROR not found") - profiles = self.raw_profile_data[self.profile_name]["outputs"] - if self.target_name not in profiles: - return red("ERROR not found") - return green("OK found") + return SubtaskStatus( + log_msg=green("OK found and valid"), + run_status=RunStatus.PASS, + details="", + summary_message="Profile is valid", + ) - def _choose_profile_names(self) -> Optional[List[str]]: + def _choose_profile_names(self) -> Tuple[List[str], str]: project_profile: Optional[str] = None if os.path.exists(self.project_path): try: @@ -224,7 +234,7 @@ def _choose_profile_names(self) -> Optional[List[str]]: args_profile: Optional[str] = getattr(self.args, "profile", None) try: - return [Profile.pick_profile_name(args_profile, project_profile)] + return [Profile.pick_profile_name(args_profile, project_profile)], "" except dbt.exceptions.DbtConfigError: pass # try to guess @@ -233,16 +243,30 @@ def _choose_profile_names(self) -> Optional[List[str]]: if self.raw_profile_data: profiles = [k for k in self.raw_profile_data if k != "config"] if project_profile is None: - self.messages.append("Could not load dbt_project.yml") + summary_message = "Could not load dbt_project.yml" elif len(profiles) == 0: - self.messages.append("The profiles.yml has no profiles") + summary_message = "The profiles.yml has no profiles" elif len(profiles) == 1: - self.messages.append(ONLY_PROFILE_MESSAGE.format(profiles[0])) + summary_message = ONLY_PROFILE_MESSAGE.format(profiles[0]) else: - self.messages.append( - MULTIPLE_PROFILE_MESSAGE.format("\n".join(" - {}".format(o) for o in profiles)) + summary_message = MULTIPLE_PROFILE_MESSAGE.format( + "\n".join(" - {}".format(o) for o in profiles) ) - return profiles + return profiles, summary_message + + def _read_adapter_version(self, module) -> Tuple[str]: + """read the version out of a standard adapter file""" + try: + version = importlib.import_module(module).version + except ModuleNotFoundError: + version = red("ERROR not found") + except Exception as exc: + version = red("ERROR {}".format(exc)) + raise dbt.exceptions.DbtInternalError( + f"Error when reading adapter version from {module}: {exc}" + ) + + return version def _choose_target_name(self, profile_name: str): has_raw_profile = ( @@ -266,72 +290,109 @@ def _choose_target_name(self, profile_name: str): ) return target_name - def _load_profile(self): - if not os.path.exists(self.profile_path): - self.profile_fail_details = FILE_NOT_FOUND - self.messages.append( - MISSING_PROFILE_MESSAGE.format(path=self.profile_path, url=ProfileConfigDocs) + # =============== + # Loading project + # =============== + + def _load_project(self) -> SubtaskStatus: + """ + Side effect: load self.project + """ + if not os.path.exists(self.project_path): + return SubtaskStatus( + log_msg=red("ERROR not found"), + run_status=RunStatus.FAIL, + details=FILE_NOT_FOUND, + summary_message=( + f"Project loading failed for the following reason:" + f"\n project path <{self.project_path}> not found" + ), ) - self.any_failure = True - return red("ERROR not found") + + renderer = DbtProjectYamlRenderer(self.profile, self.cli_vars) try: - raw_profile_data = load_yaml_text( - dbt.clients.system.load_file_contents(self.profile_path) + self.project = Project.from_project_root( + self.project_dir, + renderer, + verify_version=self.args.VERSION_CHECK, + ) + except dbt.exceptions.DbtConfigError as exc: + return SubtaskStatus( + log_msg=red("ERROR invalid"), + run_status=RunStatus.FAIL, + details=str(exc), + summary_message=( + f"Project loading failed for the following reason:" f"\n{str(exc)}" f"\n" + ), ) - except Exception: - pass # we'll report this when we try to load the profile for real else: - if isinstance(raw_profile_data, dict): - self.raw_profile_data = raw_profile_data + return SubtaskStatus( + log_msg=green("OK found and valid"), + run_status=RunStatus.PASS, + details="", + summary_message="Project is valid", + ) - profile_errors = [] - profile_names = self._choose_profile_names() - renderer = ProfileRenderer(self.cli_vars) - for profile_name in profile_names: - try: - profile: Profile = Profile.render( - renderer, - profile_name, - self.args.profile, - self.args.target, - # TODO: Generalize safe access to flags.THREADS: - # https://github.com/dbt-labs/dbt-core/issues/6259 - getattr(self.args, "threads", None), - ) - except dbt.exceptions.DbtConfigError as exc: - profile_errors.append(str(exc)) - else: - if len(profile_names) == 1: - # if a profile was specified, set it on the task - self.target_name = self._choose_target_name(profile_name) - self.profile = profile + def _profile_found(self) -> str: + if not self.raw_profile_data: + return red("ERROR not found") + assert self.raw_profile_data is not None + if self.profile_name in self.raw_profile_data: + return green("OK found") + else: + return red("ERROR not found") - if profile_errors: - self.profile_fail_details = "\n\n".join(profile_errors) - return red("ERROR invalid") - return green("OK found and valid") + def _target_found(self) -> str: + requirements = self.raw_profile_data and self.profile_name and self.target_name + if not requirements: + return red("ERROR not found") + # mypy appeasement, we checked just above + assert self.raw_profile_data is not None + assert self.profile_name is not None + assert self.target_name is not None + if self.profile_name not in self.raw_profile_data: + return red("ERROR not found") + profiles = self.raw_profile_data[self.profile_name]["outputs"] + if self.target_name not in profiles: + return red("ERROR not found") + else: + return green("OK found") + + # ============ + # Config tests + # ============ - def test_git(self): + def test_git(self) -> SubtaskStatus: try: dbt.clients.system.run_cmd(os.getcwd(), ["git", "--help"]) except dbt.exceptions.ExecutableError as exc: - self.messages.append("Error from git --help: {!s}".format(exc)) - self.any_failure = True - return red("ERROR") - return green("OK found") + return SubtaskStatus( + log_msg=red("ERROR"), + run_status=RunStatus.FAIL, + details="git error", + summary_message="Error from git --help: {!s}".format(exc), + ) + else: + return SubtaskStatus( + log_msg=green("OK found"), + run_status=RunStatus.PASS, + details="", + summary_message="git is installed and on the path", + ) - def test_dependencies(self): + def test_dependencies(self) -> List[SubtaskStatus]: fire_event(DebugCmdOut(msg="Required dependencies:")) - logline_msg = self.test_git() - fire_event(DebugCmdResult(msg=f" - git [{logline_msg}]\n")) + git_test_status = self.test_git() + fire_event(DebugCmdResult(msg=f" - git [{git_test_status.log_msg}]\n")) + + return [git_test_status] - def test_configuration(self, profile_status): + def test_configuration(self, profile_status_msg, project_status_msg): fire_event(DebugCmdOut(msg="Configuration:")) - fire_event(DebugCmdOut(msg=f" profiles.yml file [{profile_status}]")) - project_status = self._load_project() - fire_event(DebugCmdOut(msg=f" dbt_project.yml file [{project_status}]")) + fire_event(DebugCmdOut(msg=f" profiles.yml file [{profile_status_msg}]")) + fire_event(DebugCmdOut(msg=f" dbt_project.yml file [{project_status_msg}]")) # skip profile stuff if we can't find a profile name if self.profile_name is not None: @@ -346,72 +407,57 @@ def test_configuration(self, profile_status): ) ) - self._log_project_fail() - self._log_profile_fail() - - def _log_project_fail(self): - if not self.project_fail_details: - return - - self.any_failure = True - if self.project_fail_details == FILE_NOT_FOUND: - return - msg = ( - f"Project loading failed for the following reason:" - f"\n{self.project_fail_details}" - f"\n" - ) - self.messages.append(msg) - - def _log_profile_fail(self): - if not self.profile_fail_details: - return - - self.any_failure = True - if self.profile_fail_details == FILE_NOT_FOUND: - return - msg = ( - f"Profile loading failed for the following reason:" - f"\n{self.profile_fail_details}" - f"\n" - ) - self.messages.append(msg) + # =============== + # Connection test + # =============== @staticmethod - def attempt_connection(profile): - """Return a string containing the error message, or None if there was - no error. - """ + def attempt_connection(profile) -> Optional[str]: + """Return a string containing the error message, or None if there was no error.""" register_adapter(profile) adapter = get_adapter(profile) try: with adapter.connection_named("debug"): + # set in adapter repos adapter.debug_query() except Exception as exc: return COULD_NOT_CONNECT_MESSAGE.format( err=str(exc), url=ProfileConfigDocs, ) - return None - def _connection_result(self): - result = self.attempt_connection(self.profile) - if result is not None: - self.messages.append(result) - self.any_failure = True - return red("ERROR") - return green("OK connection ok") - - def test_connection(self): - if not self.profile: - return + def test_connection(self) -> SubtaskStatus: + if self.profile is None: + fire_event(DebugCmdOut(msg="Connection test skipped since no profile was found")) + return SubtaskStatus( + log_msg=red("SKIPPED"), + run_status=RunStatus.SKIP, + details="No profile found", + summary_message="Connection test skipped since no profile was found", + ) + fire_event(DebugCmdOut(msg="Connection:")) for k, v in self.profile.credentials.connection_info(): fire_event(DebugCmdOut(msg=f" {k}: {v}")) - res = self._connection_result() - fire_event(DebugCmdOut(msg=f" Connection test: [{res}]\n")) + connection_result = self.attempt_connection(self.profile) + if connection_result is not None: + status = SubtaskStatus( + log_msg=red("ERROR"), + run_status=RunStatus.FAIL, + details="Failure in connecting to db", + summary_message=connection_result, + ) + else: + status = SubtaskStatus( + log_msg=green("OK connection ok"), + run_status=RunStatus.PASS, + details="", + summary_message="Connection test passed", + ) + fire_event(DebugCmdOut(msg=f" Connection test: [{status.log_msg}]\n")) + return status @classmethod def validate_connection(cls, target_dict): diff --git a/core/dbt/version.py b/core/dbt/version.py index 923e37b40d6..a5f3ed7d2c6 100644 --- a/core/dbt/version.py +++ b/core/dbt/version.py @@ -222,6 +222,7 @@ def _get_adapter_plugin_names() -> Iterator[str]: if spec is None or spec.submodule_search_locations is None: return + print(spec.submodule_search_locations) for adapters_path in spec.submodule_search_locations: version_glob = os.path.join(adapters_path, "*", "__version__.py") for version_path in glob.glob(version_glob): @@ -232,5 +233,6 @@ def _get_adapter_plugin_names() -> Iterator[str]: yield plugin_name +# __version__ = "1.6.0b2" __version__ = "1.6.0b2" installed = get_installed_version() From bc88d1cc063d69cdf9af3400665597e39467237d Mon Sep 17 00:00:00 2001 From: Mila Page Date: Sun, 4 Jun 2023 02:58:06 -0700 Subject: [PATCH 06/16] fixes for --connection-flag --- core/dbt/task/debug.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/core/dbt/task/debug.py b/core/dbt/task/debug.py index 43f889dc25d..2f5078c85fb 100644 --- a/core/dbt/task/debug.py +++ b/core/dbt/task/debug.py @@ -60,6 +60,7 @@ FILE_NOT_FOUND = "file not found" + SubtaskStatus = namedtuple( "SubtaskStatus", ["log_msg", "run_status", "details", "summary_message"] ) @@ -128,6 +129,7 @@ def run(self): load_project_status = self._load_project() if self.args.connection: fire_event(DebugCmdOut(msg="Skipping steps before connection verification")) + dependencies_statuses = [] else: # this job's status not logged since already accounted for in _load_* commands self.test_configuration(load_profile_status.log_msg, load_project_status.log_msg) @@ -243,9 +245,9 @@ def _choose_profile_names(self) -> Tuple[List[str], str]: if self.raw_profile_data: profiles = [k for k in self.raw_profile_data if k != "config"] if project_profile is None: - summary_message = "Could not load dbt_project.yml" + summary_message = "Could not load dbt_project.yml\n" elif len(profiles) == 0: - summary_message = "The profiles.yml has no profiles" + summary_message = "The profiles.yml has no profiles\n" elif len(profiles) == 1: summary_message = ONLY_PROFILE_MESSAGE.format(profiles[0]) else: From 562453546f89b8d74ae0726fa2dffdf7038d8d09 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Sun, 4 Jun 2023 03:00:09 -0700 Subject: [PATCH 07/16] Add changelog --- .changes/unreleased/Features-20230604-025956.yaml | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changes/unreleased/Features-20230604-025956.yaml diff --git a/.changes/unreleased/Features-20230604-025956.yaml b/.changes/unreleased/Features-20230604-025956.yaml new file mode 100644 index 00000000000..658b5cfe361 --- /dev/null +++ b/.changes/unreleased/Features-20230604-025956.yaml @@ -0,0 +1,7 @@ +kind: Features +body: Revamp debug, add --connection flag, and drop --config-dir flag. Prepare for + future refactors/interface changes. +time: 2023-06-04T02:59:56.28283-07:00 +custom: + Author: versusfacit + Issue: 7104 7774 From 1ad29118b9019c05ca853462ecdcd4b6c287244e Mon Sep 17 00:00:00 2001 From: Mila Page Date: Sun, 4 Jun 2023 03:11:15 -0700 Subject: [PATCH 08/16] code cleanup based on git diff --- core/dbt/adapters/factory.py | 4 ---- core/dbt/version.py | 2 -- 2 files changed, 6 deletions(-) diff --git a/core/dbt/adapters/factory.py b/core/dbt/adapters/factory.py index 986bab664de..fffde6b487f 100644 --- a/core/dbt/adapters/factory.py +++ b/core/dbt/adapters/factory.py @@ -165,10 +165,6 @@ def get_adapter_constraint_support(self, name: Optional[str]) -> List[str]: FACTORY: AdapterContainer = AdapterContainer() -def get_adapter_plugins(name: Optional[str]) -> List[AdapterPlugin]: - return FACTORY.get_adapter_plugins(name) - - def register_adapter(config: AdapterRequiredConfig) -> None: FACTORY.register_adapter(config) diff --git a/core/dbt/version.py b/core/dbt/version.py index a5f3ed7d2c6..923e37b40d6 100644 --- a/core/dbt/version.py +++ b/core/dbt/version.py @@ -222,7 +222,6 @@ def _get_adapter_plugin_names() -> Iterator[str]: if spec is None or spec.submodule_search_locations is None: return - print(spec.submodule_search_locations) for adapters_path in spec.submodule_search_locations: version_glob = os.path.join(adapters_path, "*", "__version__.py") for version_path in glob.glob(version_glob): @@ -233,6 +232,5 @@ def _get_adapter_plugin_names() -> Iterator[str]: yield plugin_name -# __version__ = "1.6.0b2" __version__ = "1.6.0b2" installed = get_installed_version() From 80196174193875cb5c05d8bbde830f7df1f4cc1f Mon Sep 17 00:00:00 2001 From: Mila Page Date: Sun, 4 Jun 2023 03:56:22 -0700 Subject: [PATCH 09/16] Standardize the plugin functions used by DebugTask --- plugins/postgres/dbt/adapters/postgres/connections.py | 7 +++++++ plugins/postgres/dbt/adapters/postgres/impl.py | 3 +++ 2 files changed, 10 insertions(+) diff --git a/plugins/postgres/dbt/adapters/postgres/connections.py b/plugins/postgres/dbt/adapters/postgres/connections.py index cbbdd33fb38..2a1b4e13420 100644 --- a/plugins/postgres/dbt/adapters/postgres/connections.py +++ b/plugins/postgres/dbt/adapters/postgres/connections.py @@ -51,9 +51,16 @@ def _connection_keys(self): "user", "database", "schema", + "connect_timeout", + "role", "search_path", "keepalives_idle", "sslmode", + "sslcert", + "sslkey", + "sslrootcert", + "application_name", + "retries", ) diff --git a/plugins/postgres/dbt/adapters/postgres/impl.py b/plugins/postgres/dbt/adapters/postgres/impl.py index cd49defed83..9a3a0e33f08 100644 --- a/plugins/postgres/dbt/adapters/postgres/impl.py +++ b/plugins/postgres/dbt/adapters/postgres/impl.py @@ -140,3 +140,6 @@ def valid_incremental_strategies(self): Not used to validate custom strategies defined by end users. """ return ["append", "delete+insert"] + + def debug_query(self): + self.execute("select 1 as id") From e3ccb151c64c5c873a253efe4df928e285d86bf6 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Sun, 4 Jun 2023 04:53:46 -0700 Subject: [PATCH 10/16] Add coverage in test cases for the new flags and runtime logic --- .../adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py b/tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py index eb973b91728..56a7c2ba650 100644 --- a/tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py +++ b/tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py @@ -51,6 +51,13 @@ def test_ok(self, project): run_dbt(["debug"]) assert "ERROR" not in self.capsys.readouterr().out + def test_connection_flag(self, project): + run_dbt(["debug", "--connection"]) + + run_dbt(["debug", "--connection", "--target", "NONE"], expect_pass=False) + + run_dbt(["debug", "--connection", "--profiles-dir", "NONE"], expect_pass=False) + def test_nopass(self, project): run_dbt(["debug", "--target", "nopass"], expect_pass=False) self.assertGotValue(re.compile(r"\s+profiles\.yml file"), "ERROR invalid") From 92fcffbd54fa16a1b8c61514f2895de53a3a97ec Mon Sep 17 00:00:00 2001 From: Mila Page Date: Mon, 5 Jun 2023 15:35:33 -0700 Subject: [PATCH 11/16] revert 24e2d4 for move to its own pr --- core/dbt/cli/main.py | 1 + core/dbt/cli/params.py | 7 +++++++ core/dbt/clients/system.py | 12 ++++++++++++ core/dbt/events/types.proto | 11 +++++++++++ core/dbt/events/types.py | 12 ++++++++++++ core/dbt/task/debug.py | 10 +++++++++- tests/unit/test_events.py | 1 + 7 files changed, 53 insertions(+), 1 deletion(-) diff --git a/core/dbt/cli/main.py b/core/dbt/cli/main.py index 0a7608c1f70..34258c1b870 100644 --- a/core/dbt/cli/main.py +++ b/core/dbt/cli/main.py @@ -402,6 +402,7 @@ def show(ctx, **kwargs): @cli.command("debug") @click.pass_context @p.debug_connection +@p.config_dir @p.profile @p.profiles_dir_exists_false @p.project_dir diff --git a/core/dbt/cli/params.py b/core/dbt/cli/params.py index 9173f5f2af2..603c08d29d7 100644 --- a/core/dbt/cli/params.py +++ b/core/dbt/cli/params.py @@ -40,6 +40,13 @@ default=True, ) +config_dir = click.option( + "--config-dir", + envvar=None, + help="Show the configured location for the profiles.yml file and exit", + is_flag=True, +) + debug = click.option( "--debug/--no-debug", "-d/ ", diff --git a/core/dbt/clients/system.py b/core/dbt/clients/system.py index 31b1de2cbeb..66c59354b4f 100644 --- a/core/dbt/clients/system.py +++ b/core/dbt/clients/system.py @@ -323,6 +323,18 @@ def path_is_symlink(path: str) -> bool: return os.path.islink(path) +def open_dir_cmd() -> str: + # https://docs.python.org/2/library/sys.html#sys.platform + if sys.platform == "win32": + return "start" + + elif sys.platform == "darwin": + return "open" + + else: + return "xdg-open" + + def _handle_posix_cwd_error(exc: OSError, cwd: str, cmd: List[str]) -> NoReturn: if exc.errno == errno.ENOENT: message = "Directory does not exist" diff --git a/core/dbt/events/types.proto b/core/dbt/events/types.proto index d0bf28ff1d8..e91168e1ba9 100644 --- a/core/dbt/events/types.proto +++ b/core/dbt/events/types.proto @@ -2100,6 +2100,17 @@ message FinishedCleanPathsMsg { FinishedCleanPaths data = 2; } +// Z016 +message OpenCommand { + string open_cmd = 1; + string profiles_dir = 2; +} + +message OpenCommandMsg { + EventInfo info = 1; + OpenCommand data = 2; +} + // Z017 message Formatting { string msg = 1; diff --git a/core/dbt/events/types.py b/core/dbt/events/types.py index 27e178c1283..8b976cc86a1 100644 --- a/core/dbt/events/types.py +++ b/core/dbt/events/types.py @@ -2065,6 +2065,18 @@ def message(self) -> str: return "Finished cleaning all paths." +class OpenCommand(InfoLevel): + def code(self): + return "Z016" + + def message(self) -> str: + msg = f"""To view your profiles.yml file, run: + +{self.open_cmd} {self.profiles_dir}""" + + return msg + + # We use events to create console output, but also think of them as a sequence of important and # meaningful occurrences to be used for debugging and monitoring. The Formatting event helps eases # the tension between these two goals by allowing empty lines, heading separators, and other diff --git a/core/dbt/task/debug.py b/core/dbt/task/debug.py index 2f5078c85fb..c4fbe67594f 100644 --- a/core/dbt/task/debug.py +++ b/core/dbt/task/debug.py @@ -10,6 +10,7 @@ from dbt.events.functions import fire_event from dbt.events.types import ( + OpenCommand, DebugCmdOut, DebugCmdResult, ) @@ -104,8 +105,15 @@ def project_profile(self): return None return self.project.profile_name + def path_info(self): + open_cmd = dbt.clients.system.open_dir_cmd() + fire_event(OpenCommand(open_cmd=open_cmd, profiles_dir=self.profiles_dir)) + def run(self): - # Basic info + if self.args.config_dir: + self.path_info() + return True + version = get_installed_version().to_version_string(skip_matcher=True) fire_event(DebugCmdOut(msg="dbt version: {}".format(version))) fire_event(DebugCmdOut(msg="python version: {}".format(sys.version.split()[0]))) diff --git a/tests/unit/test_events.py b/tests/unit/test_events.py index 772891ad8a9..54fefe7d471 100644 --- a/tests/unit/test_events.py +++ b/tests/unit/test_events.py @@ -381,6 +381,7 @@ def test_event_codes(self): types.ConfirmCleanPath(path=""), types.ProtectedCleanPath(path=""), types.FinishedCleanPaths(), + types.OpenCommand(open_cmd="", profiles_dir=""), types.RunResultWarning(resource_type="", node_name="", path=""), types.RunResultFailure(resource_type="", node_name="", path=""), types.StatsLine(stats={"error": 0, "skip": 0, "pass": 0, "warn": 0, "total": 0}), From d925dc4c9d5f8760bad7bb5b2ba96cf6a105f620 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Mon, 5 Jun 2023 15:53:28 -0700 Subject: [PATCH 12/16] Revise changelog --- .changes/unreleased/Features-20230604-025956.yaml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.changes/unreleased/Features-20230604-025956.yaml b/.changes/unreleased/Features-20230604-025956.yaml index 658b5cfe361..a767f2d3045 100644 --- a/.changes/unreleased/Features-20230604-025956.yaml +++ b/.changes/unreleased/Features-20230604-025956.yaml @@ -1,6 +1,5 @@ kind: Features -body: Revamp debug, add --connection flag, and drop --config-dir flag. Prepare for - future refactors/interface changes. +body: Revamp debug, add --connection flag. Prepare for future refactors/interface changes. time: 2023-06-04T02:59:56.28283-07:00 custom: Author: versusfacit From dc9a741f30d7c7eac9338c4a73cffe4c97ecc876 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Tue, 6 Jun 2023 09:45:35 -0700 Subject: [PATCH 13/16] Cleanup redundant code and help logic along. --- .../unreleased/Features-20230604-025956.yaml | 2 +- core/dbt/task/debug.py | 52 +++++++++++++------ 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/.changes/unreleased/Features-20230604-025956.yaml b/.changes/unreleased/Features-20230604-025956.yaml index a767f2d3045..fb47bf4855d 100644 --- a/.changes/unreleased/Features-20230604-025956.yaml +++ b/.changes/unreleased/Features-20230604-025956.yaml @@ -3,4 +3,4 @@ body: Revamp debug, add --connection flag. Prepare for future refactors/interfac time: 2023-06-04T02:59:56.28283-07:00 custom: Author: versusfacit - Issue: 7104 7774 + Issue: 7104 diff --git a/core/dbt/task/debug.py b/core/dbt/task/debug.py index c4fbe67594f..aca3bb252f5 100644 --- a/core/dbt/task/debug.py +++ b/core/dbt/task/debug.py @@ -73,6 +73,12 @@ class RunStatus(Enum): SKIP = 3 +DEBUG_RUN_STATUS: Dict[str, bool] = { + "pass": True, + "fail": False, +} + + class DebugTask(BaseTask): def __init__(self, args, config): super().__init__(args, config) @@ -109,35 +115,42 @@ def path_info(self): open_cmd = dbt.clients.system.open_dir_cmd() fire_event(OpenCommand(open_cmd=open_cmd, profiles_dir=self.profiles_dir)) - def run(self): + def run(self) -> bool: if self.args.config_dir: self.path_info() - return True + return DEBUG_RUN_STATUS["pass"] - version = get_installed_version().to_version_string(skip_matcher=True) + version: str = get_installed_version().to_version_string(skip_matcher=True) fire_event(DebugCmdOut(msg="dbt version: {}".format(version))) fire_event(DebugCmdOut(msg="python version: {}".format(sys.version.split()[0]))) fire_event(DebugCmdOut(msg="python path: {}".format(sys.executable))) fire_event(DebugCmdOut(msg="os info: {}".format(platform.platform()))) # Load profile if possible, then load adapter info (which requires the profile) - load_profile_status = self._load_profile() + load_profile_status: SubtaskStatus = self._load_profile() fire_event(DebugCmdOut(msg="Using profiles dir at {}".format(self.profiles_dir))) fire_event(DebugCmdOut(msg="Using profiles.yml file at {}".format(self.profile_path))) fire_event(DebugCmdOut(msg="Using dbt_project.yml file at {}".format(self.project_path))) if load_profile_status.run_status == RunStatus.PASS: - adapter_type = self.profile.credentials.type - adapter_version = self._read_adapter_version( + if self.profile is not None: + adapter_type: str = self.profile.credentials.type + else: + raise dbt.exceptions.DbtInternalError( + "Profile should not be None if loading profile completed" + ) + + adapter_version: str = self._read_adapter_version( f"dbt.adapters.{adapter_type}.__version__" ) fire_event(DebugCmdOut(msg="adapter type: {}".format(adapter_type))) fire_event(DebugCmdOut(msg="adapter version: {}".format(adapter_version))) # Get project loaded to do additional checks - load_project_status = self._load_project() + load_project_status: SubtaskStatus = self._load_project() + + dependencies_statuses: List[SubtaskStatus] = [] if self.args.connection: fire_event(DebugCmdOut(msg="Skipping steps before connection verification")) - dependencies_statuses = [] else: # this job's status not logged since already accounted for in _load_* commands self.test_configuration(load_profile_status.log_msg, load_project_status.log_msg) @@ -147,17 +160,24 @@ def run(self): self.test_connection() # Log messages from any fails - all_statuses = [load_profile_status, load_project_status, *dependencies_statuses] - failure_count = sum(1 for status in all_statuses if status.run_status == RunStatus.FAIL) + all_statuses: List[SubtaskStatus] = [ + load_profile_status, + load_project_status, + *dependencies_statuses, + ] + all_failing_statuses: List[SubtaskStatus] = list( + filter(lambda status: status.run_status == RunStatus.FAIL, all_statuses) + ) + + failure_count: int = len(all_failing_statuses) if failure_count > 0: fire_event(DebugCmdResult(msg=red(f"{(pluralize(failure_count, 'check'))} failed:"))) + for status in all_failing_statuses: + fire_event(DebugCmdResult(msg=f"{status.summary_message}\n")) + return DEBUG_RUN_STATUS["fail"] else: fire_event(DebugCmdResult(msg=green("All checks passed!"))) - - for status in filter(lambda status: status.run_status == RunStatus.FAIL, all_statuses): - fire_event(DebugCmdResult(msg=f"{status.summary_message}\n")) - - return failure_count == 0 + return DEBUG_RUN_STATUS["pass"] # ============================== # Override for elsewhere in core @@ -264,7 +284,7 @@ def _choose_profile_names(self) -> Tuple[List[str], str]: ) return profiles, summary_message - def _read_adapter_version(self, module) -> Tuple[str]: + def _read_adapter_version(self, module) -> str: """read the version out of a standard adapter file""" try: version = importlib.import_module(module).version From 85f2275ef2a44a775b75a92a250df8961db014d0 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Tue, 6 Jun 2023 10:59:58 -0700 Subject: [PATCH 14/16] Add more output tests to add logic coverage and formatting. --- .../tests/adapter/dbt_debug/test_dbt_debug.py | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py b/tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py index 56a7c2ba650..6c80f7f7cbf 100644 --- a/tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py +++ b/tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py @@ -4,7 +4,7 @@ import yaml from dbt.cli.exceptions import DbtUsageException -from dbt.tests.util import run_dbt +from dbt.tests.util import run_dbt, run_dbt_and_capture MODELS__MODEL_SQL = """ seled 1 as id @@ -52,11 +52,22 @@ def test_ok(self, project): assert "ERROR" not in self.capsys.readouterr().out def test_connection_flag(self, project): - run_dbt(["debug", "--connection"]) - - run_dbt(["debug", "--connection", "--target", "NONE"], expect_pass=False) - - run_dbt(["debug", "--connection", "--profiles-dir", "NONE"], expect_pass=False) + """Testing that the --connection flag works as expected, including that output is not lost""" + _, out = run_dbt_and_capture(["debug", "--connection"]) + assert "Skipping steps before connection verification" + + _, out = run_dbt_and_capture( + ["debug", "--connection", "--target", "NONE"], expect_pass=False + ) + assert "1 check failed" in out + assert "The profile 'test' does not have a target named 'NONE'." in out + + _, out = run_dbt_and_capture( + ["debug", "--connection", "--profiles-dir", "NONE"], expect_pass=False + ) + assert "Using profiles dir at NONE" + assert "1 check failed" in out + assert "dbt looked for a profiles.yml file in NONE/profiles.yml" in out def test_nopass(self, project): run_dbt(["debug", "--target", "nopass"], expect_pass=False) From de24b6ce89c0b62afbf9cf57fed372e2e5ede5b5 Mon Sep 17 00:00:00 2001 From: Mila Page Date: Tue, 6 Jun 2023 11:35:40 -0700 Subject: [PATCH 15/16] Code review --- core/dbt/task/debug.py | 8 ++++---- .../adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/dbt/task/debug.py b/core/dbt/task/debug.py index aca3bb252f5..10d7e697367 100644 --- a/core/dbt/task/debug.py +++ b/core/dbt/task/debug.py @@ -132,12 +132,12 @@ def run(self) -> bool: fire_event(DebugCmdOut(msg="Using profiles.yml file at {}".format(self.profile_path))) fire_event(DebugCmdOut(msg="Using dbt_project.yml file at {}".format(self.project_path))) if load_profile_status.run_status == RunStatus.PASS: - if self.profile is not None: - adapter_type: str = self.profile.credentials.type - else: + if self.profile is None: raise dbt.exceptions.DbtInternalError( "Profile should not be None if loading profile completed" ) + else: + adapter_type: str = self.profile.credentials.type adapter_version: str = self._read_adapter_version( f"dbt.adapters.{adapter_type}.__version__" @@ -448,7 +448,7 @@ def attempt_connection(profile) -> Optional[str]: adapter = get_adapter(profile) try: with adapter.connection_named("debug"): - # set in adapter repos + # is defined in adapter class adapter.debug_query() except Exception as exc: return COULD_NOT_CONNECT_MESSAGE.format( diff --git a/tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py b/tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py index 6c80f7f7cbf..91ed8639c80 100644 --- a/tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py +++ b/tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py @@ -67,7 +67,7 @@ def test_connection_flag(self, project): ) assert "Using profiles dir at NONE" assert "1 check failed" in out - assert "dbt looked for a profiles.yml file in NONE/profiles.yml" in out + assert "dbt looked for a profiles.yml file in NONE" in out def test_nopass(self, project): run_dbt(["debug", "--target", "nopass"], expect_pass=False) From adc4dbc4d6a2e4423a0e5159acc0c2f5d94f060f Mon Sep 17 00:00:00 2001 From: Mila Page Date: Tue, 6 Jun 2023 13:48:47 -0700 Subject: [PATCH 16/16] Code review from karanke --- core/dbt/task/debug.py | 77 +++++++++---------- .../tests/adapter/dbt_debug/test_dbt_debug.py | 9 ++- 2 files changed, 44 insertions(+), 42 deletions(-) diff --git a/core/dbt/task/debug.py b/core/dbt/task/debug.py index 10d7e697367..9abc537ebd3 100644 --- a/core/dbt/task/debug.py +++ b/core/dbt/task/debug.py @@ -5,7 +5,7 @@ import sys from collections import namedtuple -from enum import Enum +from enum import Flag from typing import Optional, Dict, Any, List, Tuple from dbt.events.functions import fire_event @@ -19,6 +19,7 @@ from dbt.adapters.factory import get_adapter, register_adapter from dbt.config import PartialProject, Project, Profile from dbt.config.renderer import DbtProjectYamlRenderer, ProfileRenderer +from dbt.contracts.results import RunStatus from dbt.clients.yaml_helper import load_yaml_text from dbt.links import ProfileConfigDocs from dbt.ui import green, red @@ -67,16 +68,9 @@ ) -class RunStatus(Enum): - PASS = 1 - FAIL = 2 - SKIP = 3 - - -DEBUG_RUN_STATUS: Dict[str, bool] = { - "pass": True, - "fail": False, -} +class DebugRunStatus(Flag): + SUCCESS = True + FAIL = False class DebugTask(BaseTask): @@ -98,12 +92,9 @@ def __init__(self, args, config): # set by _load_* self.profile: Optional[Profile] = None - self.profile_fail_details = "" self.raw_profile_data: Optional[Dict[str, Any]] = None self.profile_name: Optional[str] = None self.project: Optional[Project] = None - self.project_fail_details = "" - self.messages: List[str] = [] @property def project_profile(self): @@ -118,7 +109,7 @@ def path_info(self): def run(self) -> bool: if self.args.config_dir: self.path_info() - return DEBUG_RUN_STATUS["pass"] + return DebugRunStatus.SUCCESS.value version: str = get_installed_version().to_version_string(skip_matcher=True) fire_event(DebugCmdOut(msg="dbt version: {}".format(version))) @@ -131,7 +122,7 @@ def run(self) -> bool: fire_event(DebugCmdOut(msg="Using profiles dir at {}".format(self.profiles_dir))) fire_event(DebugCmdOut(msg="Using profiles.yml file at {}".format(self.profile_path))) fire_event(DebugCmdOut(msg="Using dbt_project.yml file at {}".format(self.project_path))) - if load_profile_status.run_status == RunStatus.PASS: + if load_profile_status.run_status == RunStatus.Success: if self.profile is None: raise dbt.exceptions.DbtInternalError( "Profile should not be None if loading profile completed" @@ -166,7 +157,7 @@ def run(self) -> bool: *dependencies_statuses, ] all_failing_statuses: List[SubtaskStatus] = list( - filter(lambda status: status.run_status == RunStatus.FAIL, all_statuses) + filter(lambda status: status.run_status == RunStatus.Error, all_statuses) ) failure_count: int = len(all_failing_statuses) @@ -174,10 +165,10 @@ def run(self) -> bool: fire_event(DebugCmdResult(msg=red(f"{(pluralize(failure_count, 'check'))} failed:"))) for status in all_failing_statuses: fire_event(DebugCmdResult(msg=f"{status.summary_message}\n")) - return DEBUG_RUN_STATUS["fail"] + return DebugRunStatus.FAIL.value else: fire_event(DebugCmdResult(msg=green("All checks passed!"))) - return DEBUG_RUN_STATUS["pass"] + return DebugRunStatus.SUCCESS.value # ============================== # Override for elsewhere in core @@ -192,13 +183,14 @@ def interpret_results(self, results): def _load_profile(self) -> SubtaskStatus: """ - Side effect: load self.profile - Side effect: load self.target_name + Side effects: load self.profile + load self.target_name + load self.raw_profile_data """ if not os.path.exists(self.profile_path): return SubtaskStatus( log_msg=red("ERROR not found"), - run_status=RunStatus.FAIL, + run_status=RunStatus.Error, details=FILE_NOT_FOUND, summary_message=MISSING_PROFILE_MESSAGE.format( path=self.profile_path, url=ProfileConfigDocs @@ -235,15 +227,18 @@ def _load_profile(self) -> SubtaskStatus: details = "\n\n".join(profile_errors) return SubtaskStatus( log_msg=red("ERROR invalid"), - run_status=RunStatus.FAIL, + run_status=RunStatus.Error, details=details, - summary_message=summary_message - + (f"Profile loading failed for the following reason:" f"\n{details}" f"\n"), + summary_message=( + summary_message + f"Profile loading failed for the following reason:" + f"\n{details}" + f"\n" + ), ) else: return SubtaskStatus( log_msg=green("OK found and valid"), - run_status=RunStatus.PASS, + run_status=RunStatus.Success, details="", summary_message="Profile is valid", ) @@ -331,7 +326,7 @@ def _load_project(self) -> SubtaskStatus: if not os.path.exists(self.project_path): return SubtaskStatus( log_msg=red("ERROR not found"), - run_status=RunStatus.FAIL, + run_status=RunStatus.Error, details=FILE_NOT_FOUND, summary_message=( f"Project loading failed for the following reason:" @@ -350,7 +345,7 @@ def _load_project(self) -> SubtaskStatus: except dbt.exceptions.DbtConfigError as exc: return SubtaskStatus( log_msg=red("ERROR invalid"), - run_status=RunStatus.FAIL, + run_status=RunStatus.Error, details=str(exc), summary_message=( f"Project loading failed for the following reason:" f"\n{str(exc)}" f"\n" @@ -359,7 +354,7 @@ def _load_project(self) -> SubtaskStatus: else: return SubtaskStatus( log_msg=green("OK found and valid"), - run_status=RunStatus.PASS, + run_status=RunStatus.Success, details="", summary_message="Project is valid", ) @@ -399,14 +394,14 @@ def test_git(self) -> SubtaskStatus: except dbt.exceptions.ExecutableError as exc: return SubtaskStatus( log_msg=red("ERROR"), - run_status=RunStatus.FAIL, + run_status=RunStatus.Error, details="git error", summary_message="Error from git --help: {!s}".format(exc), ) else: return SubtaskStatus( log_msg=green("OK found"), - run_status=RunStatus.PASS, + run_status=RunStatus.Success, details="", summary_message="git is installed and on the path", ) @@ -462,7 +457,7 @@ def test_connection(self) -> SubtaskStatus: fire_event(DebugCmdOut(msg="Connection test skipped since no profile was found")) return SubtaskStatus( log_msg=red("SKIPPED"), - run_status=RunStatus.SKIP, + run_status=RunStatus.Skipped, details="No profile found", summary_message="Connection test skipped since no profile was found", ) @@ -472,20 +467,20 @@ def test_connection(self) -> SubtaskStatus: fire_event(DebugCmdOut(msg=f" {k}: {v}")) connection_result = self.attempt_connection(self.profile) - if connection_result is not None: - status = SubtaskStatus( - log_msg=red("ERROR"), - run_status=RunStatus.FAIL, - details="Failure in connecting to db", - summary_message=connection_result, - ) - else: + if connection_result is None: status = SubtaskStatus( log_msg=green("OK connection ok"), - run_status=RunStatus.PASS, + run_status=RunStatus.Success, details="", summary_message="Connection test passed", ) + else: + status = SubtaskStatus( + log_msg=red("ERROR"), + run_status=RunStatus.Error, + details="Failure in connecting to db", + summary_message=connection_result, + ) fire_event(DebugCmdOut(msg=f" Connection test: [{status.log_msg}]\n")) return status diff --git a/tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py b/tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py index 91ed8639c80..3ad39e9c2ab 100644 --- a/tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py +++ b/tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py @@ -54,7 +54,7 @@ def test_ok(self, project): def test_connection_flag(self, project): """Testing that the --connection flag works as expected, including that output is not lost""" _, out = run_dbt_and_capture(["debug", "--connection"]) - assert "Skipping steps before connection verification" + assert "Skipping steps before connection verification" in out _, out = run_dbt_and_capture( ["debug", "--connection", "--target", "NONE"], expect_pass=False @@ -118,3 +118,10 @@ def test_invalid_project_outside_current_dir(self, project): run_dbt(["debug", "--project-dir", "custom"], expect_pass=False) splitout = self.capsys.readouterr().out.split("\n") self.check_project(splitout) + + def test_profile_not_found(self, project): + _, out = run_dbt_and_capture( + ["debug", "--connection", "--profile", "NONE"], expect_pass=False + ) + assert "Profile loading failed for the following reason" in out + assert "Could not find profile named 'NONE'" in out