Skip to content

Commit

Permalink
Code review from karanke
Browse files Browse the repository at this point in the history
  • Loading branch information
@VersusFacit
VersusFacit committed Jun 6, 2023
1 parent de24b6c commit 3643378
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 35 deletions.
63 changes: 29 additions & 34 deletions core/dbt/task/debug.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import sys

from collections import namedtuple
from enum import Enum
from typing import Optional, Dict, Any, List, Tuple

from dbt.events.functions import fire_event
Expand All @@ -19,6 +18,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
Expand Down Expand Up @@ -67,12 +67,6 @@
)


class RunStatus(Enum):
PASS = 1
FAIL = 2
SKIP = 3


DEBUG_RUN_STATUS: Dict[str, bool] = {
"pass": True,
"fail": False,
Expand All @@ -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):
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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",
)
Expand Down Expand Up @@ -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:"
Expand All @@ -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"
Expand All @@ -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",
)
Expand Down Expand Up @@ -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",
)
Expand Down Expand Up @@ -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",
)
Expand All @@ -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

Expand Down
9 changes: 8 additions & 1 deletion tests/adapter/dbt/tests/adapter/dbt_debug/test_dbt_debug.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

0 comments on commit 3643378

Please sign in to comment.