From 1c15b443ea4154a5192a7da0921afc2586b0baf5 Mon Sep 17 00:00:00 2001 From: Travis Hathaway Date: Mon, 2 Jan 2023 10:21:39 +0100 Subject: [PATCH] Refactoring exception handling With this commit I wanted to make sure that modules weren't raising ClickExceptions and instead were raising their own exception types to reducing coupling. --- latz/cli.py | 7 +++++-- latz/commands/config/commands.py | 10 +++++----- latz/commands/search.py | 6 +++++- latz/config/__init__.py | 7 ++++++- latz/config/main.py | 21 ++++++++++++--------- latz/plugins/image/unsplash.py | 21 +++++++++++---------- 6 files changed, 44 insertions(+), 28 deletions(-) diff --git a/latz/cli.py b/latz/cli.py index 8694b16..09adcc4 100644 --- a/latz/cli.py +++ b/latz/cli.py @@ -5,7 +5,7 @@ from pydantic import create_model, validator from .commands import search_command, config_group -from .config import get_app_config, BaseAppConfig +from .config import get_app_config, BaseAppConfig, ConfigError from .constants import CONFIG_FILES from .plugins.manager import get_plugin_manager @@ -45,7 +45,10 @@ def cli(ctx): # Creates the actual config object which parses all possible configuration sources # listed in ``CONFIG_FILES``. - app_config = get_app_config(CONFIG_FILES, AppConfig) + try: + app_config = get_app_config(CONFIG_FILES, AppConfig) + except ConfigError as exc: + raise click.ClickException(str(exc)) # Attach several properties to click's ``ctx`` object so that we have access to it # in our sub-commands. diff --git a/latz/commands/config/commands.py b/latz/commands/config/commands.py index 5136961..29c0d0d 100644 --- a/latz/commands/config/commands.py +++ b/latz/commands/config/commands.py @@ -4,7 +4,7 @@ from rich import print as rprint from ...constants import CONFIG_FILE_CWD, CONFIG_FILE_HOME_DIR -from ...config.main import parse_config_file_as_json, write_config_file +from ...config import parse_config_file_as_json, write_config_file, ConfigError from .validators import validate_and_parse_config_values @@ -42,10 +42,10 @@ def set_command(home, config_values): # Merge the new values and old values; new overwrites the old new_config_file_data = {**parsed_config.data, **config_values} - error = write_config_file(new_config_file_data, config_file) - - if error: - raise click.ClickException(error) + try: + write_config_file(new_config_file_data, config_file) + except ConfigError as exc: + raise click.ClickException(str(exc)) group.add_command(show_command) diff --git a/latz/commands/search.py b/latz/commands/search.py index b72dd65..7571e88 100644 --- a/latz/commands/search.py +++ b/latz/commands/search.py @@ -2,6 +2,7 @@ from typing import cast, ContextManager, Any import click +import httpx from rich import print as rprint from ..image import ImageAPI @@ -19,7 +20,10 @@ def command(ctx, query: str): ) with image_api_context_manager(ctx.obj.config) as api: - result_set = api.search(query) + try: + result_set = api.search(query) + except httpx.HTTPError as exc: + raise click.ClickException(str(exc)) for res in result_set.results: rprint(res) diff --git a/latz/config/__init__.py b/latz/config/__init__.py index 929fa2b..bcdc896 100644 --- a/latz/config/__init__.py +++ b/latz/config/__init__.py @@ -1,2 +1,7 @@ -from .main import get_app_config # noqa: F401 +from .main import ( # noqa: F401 + get_app_config, + parse_config_file_as_json, + write_config_file, + ConfigError, +) from .models import BaseAppConfig # noqa: F401 diff --git a/latz/config/main.py b/latz/config/main.py index b0f807c..7db5baf 100644 --- a/latz/config/main.py +++ b/latz/config/main.py @@ -8,7 +8,6 @@ from typing import NamedTuple, Any from pydantic import ValidationError -from click import ClickException from .models import BaseAppConfig from .errors import format_validation_error, format_all_validation_errors @@ -16,6 +15,10 @@ logger = logging.getLogger(__name__) +class ConfigError(Exception): + pass + + class ParsedConfigFile(NamedTuple): #: Path to the configuration file path: Path @@ -123,11 +126,11 @@ def get_app_config( Given a sequence of ``paths`` first attempts to parse these as JSON and then attempts to parse valid JSON objects as ``AppConfig`` objects. - :raises ClickException: Happens when any errors are encountered during config parsing + :raises ConfigError: Happens when any errors are encountered during config parsing """ parsed_config_files = parse_config_files(paths) - # No files were found 🤷‍ + # No files were found 🤷‍; let's return a default config object if parsed_config_files is None: return model_class() @@ -140,7 +143,7 @@ def get_app_config( # Fail loudly if any errors if len(errors) > 0: - raise ClickException(format_all_validation_errors(errors)) + raise ConfigError(format_all_validation_errors(errors)) # Gather configs app_configs = tuple(parsed.model for parsed in parsed_config_files if parsed.model) @@ -153,14 +156,14 @@ def get_app_config( return merge_app_configs(app_configs, model_class) -def write_config_file( - config_file_data: dict[str, Any], config_file: Path -) -> str | None: +def write_config_file(config_file_data: dict[str, Any], config_file: Path) -> None: """ - Attempts to write a + Attempts to write config file and returns the exception as a string if it failed. + + :raises ConfigError: Raised when we are not able to write our config file. """ try: with config_file.open("w") as fp: json.dump(config_file_data, fp, indent=2) except OSError as exc: - return str(exc) + raise ConfigError(str(exc)) diff --git a/latz/plugins/image/unsplash.py b/latz/plugins/image/unsplash.py index 6a9d7dc..7f0ce47 100644 --- a/latz/plugins/image/unsplash.py +++ b/latz/plugins/image/unsplash.py @@ -2,8 +2,7 @@ import urllib.parse -import click -from httpx import Client, HTTPError, Headers +from httpx import Client, Headers from pydantic import BaseModel, Field from ...image import ( @@ -47,24 +46,26 @@ class UnsplashImageAPI(ImageAPI): #: Base URL for the Unsplash API base_url = "https://api.unsplash.com/" - def __init__(self, client_id: str, client): + #: Endpoint used for searching images + search_endpoint = "/search/photos" + + def __init__(self, client_id: str, client: Client): """We use this initialization method to properly configure the ``httpx.Client`` object""" self._client_id = client_id self._headers = {"Authorization": f"Client-ID {client_id}"} - self._client: Client = client + self._client = client self._client.headers = Headers(self._headers) def search(self, query: str) -> ImageSearchResultSet: """ Find images based on a ``search_term`` and return an ``ImageSearchResultSet`` + + :raises HTTPError: Encountered during problems querying the API """ - search_url = urllib.parse.urljoin(self.base_url, "/search/photos") + search_url = urllib.parse.urljoin(self.base_url, self.search_endpoint) - try: - resp = self._client.get(search_url, params={"query": query}) - resp.raise_for_status() - except HTTPError as exc: - raise click.ClickException(str(exc)) + resp = self._client.get(search_url, params={"query": query}) + resp.raise_for_status() json_data = resp.json()