From 9486161054b269bce3e5fe24282056ee0105a6f9 Mon Sep 17 00:00:00 2001 From: Nicola Coretti Date: Mon, 7 Aug 2023 10:21:29 +0200 Subject: [PATCH] Add new API for error reporting (#20) --- doc/changes/changes_0.4.0.md | 1 + exasol/error/__init__.py | 3 + exasol/error/_error.py | 95 ++++++++++++++++++++++ exasol/error/version.py | 5 ++ exasol_error_reporting_python/__init__.py | 18 ++++- pyproject.toml | 4 + test/unit/error_test.py | 99 +++++++++++++++++++++++ 7 files changed, 224 insertions(+), 1 deletion(-) create mode 100644 exasol/error/__init__.py create mode 100644 exasol/error/_error.py create mode 100644 exasol/error/version.py create mode 100644 test/unit/error_test.py diff --git a/doc/changes/changes_0.4.0.md b/doc/changes/changes_0.4.0.md index 39f4286..2ee7965 100644 --- a/doc/changes/changes_0.4.0.md +++ b/doc/changes/changes_0.4.0.md @@ -8,4 +8,5 @@ T.B.D ### Refactoring + - #19: Rework error reporting API - #17: Removed setup.py and updated poetry in actions \ No newline at end of file diff --git a/exasol/error/__init__.py b/exasol/error/__init__.py new file mode 100644 index 0000000..b7afcb0 --- /dev/null +++ b/exasol/error/__init__.py @@ -0,0 +1,3 @@ +from exasol.error._error import ExaError, Parameter + +__all__ = ["ExaError", "Parameter"] diff --git a/exasol/error/_error.py b/exasol/error/_error.py new file mode 100644 index 0000000..dbb27f3 --- /dev/null +++ b/exasol/error/_error.py @@ -0,0 +1,95 @@ +import warnings +from collections.abc import Iterable, Mapping +from dataclasses import dataclass +from typing import Dict, List, Union + +with warnings.catch_warnings(): + warnings.simplefilter("ignore") + from exasol_error_reporting_python import exa_error + + +@dataclass(frozen=True) +class Parameter: + value: str + description: Union[None, str] + + +class Error: + def __init__( + self, + name: str, + message: str, + mitigations: Union[str, Iterable[str]], + parameters: Dict[str, Union[str, Parameter]], + ): + # This function maybe flattened into or moved out of the constructor in the future. + def build_error(code, msg, mitigations, params): + builder = exa_error.ExaError.message_builder(code) + builder.message(msg) + + for mitigation in mitigations: + builder.mitigation(mitigation) + + for k, v in params.items(): + name, value, description = k, v, "" + if isinstance(v, Parameter): + value = v.value + description = v.description + builder.parameter(name, value, description) + + return builder + + self._error = build_error(name, message, mitigations, parameters) + + def __str__(self) -> str: + return f"{self._error}" + + # TODO: Implement __format__ to conveniently support long and short reporting format + + +def ExaError( + name: str, + message: str, + mitigations: Union[str, List[str]], + parameters: Mapping[str, Union[str, Parameter]], +) -> Error: + """Create a new ExaError. + + The caller is responsible to make sure that the chosen name/id is unique. + + Args: + name: unique name/id of the error. + message: the error message itself. It can contain placeholders for parameters. + mitigations: One or multiple mitigations strings. A mitigation can contain placeholders for parameters. + parameters: which will be used for substitute the parameters in the mitigation(s) and the error message. + + + Returns: + An error type containing all relevant information. + + + Raises: + This function should not raise under any circumstances. + In case of error, it should report it also as an error type which is returned by this function. + Still the returned error should contain a references or information about the original call. + + Attention: + + FIXME: Due to legacy reasons this function currently still may raise an `ValueError` (Refactoring Required). + + Potential error scenarios which should taken into account are the following ones: + * E-ERP-1: Invalid error code provided + params: + * Original ErrorCode + * Original error message + * Original mitigations + * Original parameters + * E-ERP-2 Unknown error/exception occurred + params: + * exception/msg + * Original ErrorCode + * Original error message + * Original mitigations + * Original parameters + """ + return Error(name, message, mitigations, parameters) diff --git a/exasol/error/version.py b/exasol/error/version.py new file mode 100644 index 0000000..b922350 --- /dev/null +++ b/exasol/error/version.py @@ -0,0 +1,5 @@ +MAJOR = 0 +MINOR = 4 +PATCH = 0 + +VERSION = f"{MAJOR}.{MINOR}.{PATCH}" diff --git a/exasol_error_reporting_python/__init__.py b/exasol_error_reporting_python/__init__.py index 7fd229a..53cc48f 100644 --- a/exasol_error_reporting_python/__init__.py +++ b/exasol_error_reporting_python/__init__.py @@ -1 +1,17 @@ -__version__ = '0.2.0' +import warnings + + +class ErrorReportingDeprecationWarning(DeprecationWarning): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + + pass + + +warnings.warn( + f"The package {__name__} is deprecated, please use the 'exasol.error' package instead.", + ErrorReportingDeprecationWarning, + stacklevel=2, +) + +__version__ = "0.4.0" diff --git a/pyproject.toml b/pyproject.toml index f076e89..05d8def 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,5 +1,9 @@ [tool.poetry] name = "exasol-error-reporting-python" +packages = [ + {include = "exasol"}, + {include = "exasol_error_reporting_python"} +] version = "0.4.0" description = "Exasol Python Error Reporting" license = "MIT" diff --git a/test/unit/error_test.py b/test/unit/error_test.py new file mode 100644 index 0000000..3e0f25d --- /dev/null +++ b/test/unit/error_test.py @@ -0,0 +1,99 @@ +import importlib +from collections import namedtuple +from contextlib import contextmanager +from inspect import cleandoc + +import pytest + +from exasol.error import ExaError, Parameter + +Data = namedtuple("Data", ["code", "message", "mitigations", "parameters"]) + + +@contextmanager +def does_not_raise(): + try: + yield + except Exception as ex: + pytest.fail(f"Exception was raised where none is to be expected, details: {ex}") + + +@pytest.mark.parametrize( + "expected,data", + [ + ( + cleandoc( + """ + E-TEST-1: Not enough space on device '/dev/sda1'. Known mitigations: + * Delete something from '/dev/sda1'. + * Create larger partition. + """ + ), + Data( + code="E-TEST-1", + message="Not enough space on device {{device}}.", + mitigations=[ + "Delete something from {{device}}.", + "Create larger partition.", + ], + parameters={"device": Parameter("/dev/sda1", "name of the device")}, + ), + ), + ( + cleandoc( + """ + E-TEST-1: Not enough space on device '/dev/sda1'. Known mitigations: + * Delete something from '/dev/sda1'. + * Create larger partition. + """ + ), + Data( + code="E-TEST-1", + message="Not enough space on device {{device}}.", + mitigations=[ + "Delete something from {{device}}.", + "Create larger partition.", + ], + parameters={"device": "/dev/sda1"}, + ), + ), + ], +) +def test_exa_error_as_string(expected, data): + error = ExaError(data.code, data.message, data.mitigations, data.parameters) + actual = str(error) + assert actual == expected + + +@pytest.mark.xfail( + True, + reason="Old implementation does not avoid throwing exceptions, further refactoring is needed.", +) +@pytest.mark.parametrize( + "data", + [ + ( + Data( + code="BROKEN_ERROR_CODE", + message='"Not enough space on device {{device}}."', + mitigations=[ + "Delete something from {{device}}.", + "Create larger partition.", + ], + parameters={"device": Parameter("/dev/sda1", "name of the device")}, + ) + ), + ], +) +def test_exa_error_does_not_throw_error_on_invalid(data): + with does_not_raise(): + _ = ExaError(data.code, data.message, data.mitigations, data.parameters) + + +def test_using_the_old_api_produces_deprecation_warning(): + with pytest.warns(DeprecationWarning): + # due to the fact that the exasol.error package already imports the package at an earlier stage + # we need to ensure the module is imported during the test itself. + import exasol_error_reporting_python + + importlib.reload(exasol_error_reporting_python)