Added custom exceptions, reworked Result API - raising exception righ…

…t away

arca/_arca.py

@@ -9,6 +9,7 @@
 from dogpile.cache import make_region, CacheRegion
 from git import Repo
 
+from .exceptions import ArcaMisconfigured
 from .backend import BaseBackend
 from .result import Result
 from .task import Task
@@ -52,7 +53,7 @@ def _get_backend_instance(self, backend: BackendDefinitionType) -> BaseBackend:
             backend = backend()
 
         if not issubclass(backend.__class__, BaseBackend):
-            raise ValueError(f"{backend.__class__} is not an subclass of BaseBackend")
+            raise ArcaMisconfigured(f"{backend.__class__} is not an subclass of BaseBackend")
 
         return backend
 
@@ -72,9 +73,11 @@ def _make_region(self) -> CacheRegion:
         arguments = self.get_setting("cache_backend_arguments", None)
 
         if isinstance(arguments, str) and arguments:
-            arguments = json.loads(arguments)  # TODO: catch errors and raise custom exception
+            try:
+                arguments = json.loads(arguments)
+            except ValueError:
+                raise ArcaMisconfigured("Cache backend arguments couldn't be converted to a dictionary.")
 
-        # TODO: custom exceptions
         try:
             region = make_region().configure(
                 self.get_setting("cache_backend", "dogpile.cache.null"),
@@ -83,17 +86,16 @@ def _make_region(self) -> CacheRegion:
             )
             region.set("last_arca_run", datetime.now().isoformat())
         except ModuleNotFoundError:
-            raise ValueError("Cache backend cannot load a required library")
+            raise ModuleNotFoundError("Cache backend cannot load a required library.")
         except Exception as e:
-            raise ValueError("The provided cache is not working - probably misconfigured. ")
+            raise ArcaMisconfigured("The provided cache is not working - most likely misconfigured.")
 
         return region
 
     def validate_repo_url(self, repo: str):
         # that should match valid git repos
         if not isinstance(repo, str) or not re.match(r"^(https?|file)://[\w._\-/~]*[.git]?/?$", repo):
-            # TODO: probably a custom exception would be better
-            raise ValueError(f"{repo} is not a valid http[s] or file:// git repo")
+            raise ValueError(f"{repo} is not a valid http[s] or file:// git repository.")
 
     def repo_id(self, repo: str) -> str:
         if repo.startswith("http"):
@@ -148,7 +150,7 @@ def pull_again(self, repo: Optional[str]=None, branch: Optional[str]=None) -> No
         if repo is None and branch is None:
             self._current_hash = {}
         elif repo is None:
-            raise ValueError("You can't define just the branch to pull again.")  # TODO: custom exception
+            raise ValueError("You can't define just the branch to pull again.")
         elif branch is None and repo is not None:
             self._current_hash.pop(self.repo_id(repo), None)
         else:
@@ -196,13 +198,9 @@ def run(self, repo: str, branch: str, task: Task) -> Result:
         def create_value():
             return self.backend.run(repo, branch, task, git_repo, repo_path)
 
-        def should_cache(value: Result):
-            return value.success
-
         return self.region.get_or_create(
             self.cache_key(repo, branch, task, git_repo),
-            create_value,
-            should_cache_fn=should_cache
+            create_value
         )
 
     def static_filename(self, repo: str, branch: str, relative_path: Union[str, Path]) -> Path:

arca/backend/docker.py

@@ -18,6 +18,7 @@
 from requests.exceptions import ConnectionError
 
 import arca
+from arca.exceptions import ArcaMisconfigured, BuildError, PushToRegistryError
 from arca.result import Result
 from arca.task import Task
 from arca.utils import logger, LazySettingProperty
@@ -46,20 +47,19 @@ def validate_settings(self):
         super().validate_settings()
 
         if self.inherit_image is not None and self.get_dependencies() is not None:
-            # TODO: Custom Exception
-            raise ValueError("An external image is used as a base, therefore Arca can't install dependencies")
+            raise ArcaMisconfigured("An external image is used as a base, therefore Arca can't install dependencies")
 
         if self.inherit_image is not None:
             try:
                 assert len(str(self.inherit_image).split(":")) == 2
             except (ValueError, AssertionError):
-                raise ValueError("Image which should be inherited is not in the proper docker format")
+                raise ArcaMisconfigured("Image which should be inherited is not in the proper docker format")
 
         if self.push_to_registry_name is not None:
             try:
                 assert 2 >= len(str(self.inherit_image).split("/")) <= 3
             except ValueError:
-                raise ValueError("Repository name where images should be pushed doesn't match the format")
+                raise ArcaMisconfigured("Repository name where images should be pushed doesn't match the format")
 
     def check_docker_access(self):
         """ Checks if the current user can access docker, raises exception otherwise
@@ -70,8 +70,7 @@ def check_docker_access(self):
             self.client.ping()  # check that docker is running and user is permitted to access it
         except ConnectionError as e:
             logger.exception(e)
-            # TODO: Custom exception
-            raise ValueError("Docker is not running or the current user doesn't have permissions to access docker.")
+            raise BuildError("Docker is not running or the current user doesn't have permissions to access docker.")
 
     def get_dependencies(self) -> Optional[List[str]]:
         """ Returns a converted list of dependencies.
@@ -85,8 +84,7 @@ def get_dependencies(self) -> Optional[List[str]]:
         try:
             dependencies = list([str(x) for x in self.apk_dependencies])
         except (TypeError, ValueError):
-            # TODO: Custom exception
-            raise ValueError("Apk dependencies can't be converted into a list of strings")
+            raise ArcaMisconfigured("Apk dependencies can't be converted into a list of strings")
 
         if not len(dependencies):
             return None
@@ -257,7 +255,7 @@ def pull_or_get(self, name, tag):
         try:
             self.client.images.pull(name, tag)
         except docker.errors.APIError:
-            raise ValueError("The specified image from which Arca should inherit can't be pulled")  # TODO: Custom
+            raise ArcaMisconfigured("The specified image from which Arca should inherit can't be pulled")
         return name, tag
 
     def create_image(self, image_name: str, image_tag: str,
@@ -355,8 +353,7 @@ def push_to_registry(self, image: Image, image_tag: str):
         last_line = json.loads(result.split("\n")[-1])
 
         if "error" in last_line:
-            # TODO: Custom exception (with full output)
-            raise ValueError(f"Push of the image failed because of: {last_line['error']}")
+            raise PushToRegistryError(f"Push of the image failed because of: {last_line['error']}", full_output=result)
 
         logger.info("Pushed image to registry %s:%s", self.push_to_registry_name, image_tag)
         logger.debug("Info:\n%s", result)
@@ -485,7 +482,9 @@ def run(self, repo: str, branch: str, task: Task, git_repo: Repo, repo_path: Pat
             return Result(json.loads(res))
         except Exception as e:
             logger.exception(e)
-            return Result({"success": False, "error": str(e)})
+            raise BuildError("The build failed", extra_info={
+                "exception": e
+            })
         finally:
             if not self.keep_container_running:
                 container.kill(signal.SIGKILL)

arca/backend/venv.py

@@ -1,7 +1,6 @@
 import json
 import os
 import stat
-import traceback
 from pathlib import Path
 from venv import EnvBuilder
 
@@ -12,6 +11,7 @@
 from arca.task import Task
 from arca.result import Result
 from arca.utils import logger
+from arca.exceptions import BuildError
 from .base import BaseBackend
 
 
@@ -44,17 +44,21 @@ def create_or_get_venv(self, path: Path):
                                            stdout=subprocess.PIPE, stderr=subprocess.PIPE)
 
                 [out_stream, err_stream] = process.communicate()
+                out_stream = out_stream.decode("utf-8")
+                err_stream = err_stream.decode("utf-8")
 
                 logger.debug("Return code is %s", process.returncode)
-                logger.debug(out_stream.decode("utf-8"))
-                logger.debug(err_stream.decode("utf-8"))
+                logger.debug(out_stream)
+                logger.debug(err_stream)
 
                 if process.returncode:
                     venv_path.rmdir()
-                    raise ValueError("Unable to install requirements.txt")  # TODO: custom exception
+                    raise BuildError("Unable to install requirements.txt", extra_info={
+                        "out_stream": out_stream,
+                        "err_stream": err_stream,
+                        "returncode": process.returncode
+                    })
 
-                logger.debug(out_stream.decode("utf-8"))
-                logger.debug(err_stream.decode("utf-8"))
             else:
                 logger.info("Requirements file not present in repo, empty venv it is.")
         else:
@@ -96,6 +100,8 @@ def run(self, repo: str, branch: str, task: Task, git_repo: Repo, repo_path: Pat
             return Result(json.loads(out_stream.decode("utf-8")))
         except Exception as e:
             logger.exception(e)
-            return Result({"success": False, "error": (traceback.format_exc() + "\n" +
-                                                       out_stream.decode("utf-8") + "\n\n" +
-                                                       err_stream.decode("utf-8"))})
+            raise BuildError("The build failed", extra_info={
+                "exception": e,
+                "out_stream": out_stream,
+                "err_stream": err_stream,
+            })

arca/exceptions.py

@@ -0,0 +1,26 @@
+
+
+class ArcaException(Exception):
+    pass
+
+
+class ArcaMisconfigured(ArcaException):
+    pass
+
+
+class TaskMisconfigured(ValueError, ArcaException):
+    pass
+
+
+class BuildError(ArcaException):
+
+    def __init__(self, *args, extra_info=None, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.extra_info = extra_info
+
+
+class PushToRegistryError(ArcaException):
+
+    def __init__(self, *args, full_output=None,  **kwargs):
+        super().__init__(*args, **kwargs)
+        self.full_output = full_output

arca/result.py

@@ -1,23 +1,14 @@
 from typing import Dict, Union, Any
 
+from arca.exceptions import BuildError
+
 
 class Result:
 
     def __init__(self, result: Dict[str, Union[bool, str, Any]]) -> None:
-        self.success = result.get("success")
-        self._result = result.get("result")
-        self._error = result.get("error")
-
-    @property
-    def error(self) -> str:
-        if self.success:
-            raise AttributeError("The task succeeded, there's no error")
-
-        return self._error
-
-    @property
-    def result(self) -> Any:
-        if not self.success:
-            raise AttributeError("The task did not succeed, there's not result")
+        if not result.get("success"):
+            raise BuildError("The build failed", extra_info={
+                "traceback": result.get("error")
+            })
 
-        return self._result
+        self.output = result.get("result")

arca/task.py

@@ -5,6 +5,8 @@
 from pathlib import Path
 from typing import Optional, Tuple, Any, Dict, Iterable
 
+from .exceptions import TaskMisconfigured
+
 
 class Task:
 
@@ -14,7 +16,7 @@ def __init__(self, function_call: str, *,
                  args: Optional[Iterable[Any]]=None,
                  kwargs: Optional[Dict[str, Any]]=None) -> None:
         if re.match(r".*\s.*", function_call):
-            raise ValueError("function_call contains a whitespace")  # TODO: custom exception
+            raise TaskMisconfigured("function_call contains a whitespace")
 
         self.function_call = function_call
         self.imports = list(imports or [])

arca/utils.py

@@ -2,6 +2,9 @@
 import logging
 from typing import Any, Dict, Optional
 
+from .exceptions import ArcaMisconfigured
+
+
 NOT_SET = object()
 logger = logging.getLogger("arca")
 logger.setLevel(logging.DEBUG)
@@ -10,12 +13,14 @@
 def load_class(location: str) -> type:
     module_name = ".".join(location.split(".")[:-1])
     class_name = location.split(".")[-1]
-    imported_module = importlib.import_module(module_name)
 
     try:
+        imported_module = importlib.import_module(module_name)
         return getattr(imported_module, class_name)
+    except ModuleNotFoundError:
+        raise ArcaMisconfigured(f"{module_name} does not exist.")
     except AttributeError:
-        raise ValueError(f"{module_name} does not have a {class_name} class")  # TODO: custom exception?
+        raise ArcaMisconfigured(f"{module_name} does not have a {class_name} class")
 
 
 class LazySettingProperty:

tests/backend/test_backends.py

@@ -7,6 +7,7 @@
 from git import Repo
 
 from arca import Arca, VenvBackend, DockerBackend, Task
+from arca.exceptions import BuildError
 
 if os.environ.get("TRAVIS", False):
     BASE_DIR = "/home/travis/build/{}/test_loc".format(os.environ.get("TRAVIS_REPO_SLUG", "mikicz/arca"))
@@ -77,8 +78,7 @@ def test_backends(backend, requirements_location, file_location):
 
     result = arca.run(f"file://{git_dir}", "master", task)
 
-    assert result.success
-    assert result.result == "Some string"
+    assert result.output == "Some string"
 
     with filepath.open("w") as fl:
         fl.write(SECOND_RETURN_STR_FUNCTION)
@@ -89,13 +89,11 @@ def test_backends(backend, requirements_location, file_location):
     repo.index.commit("Updated function")
 
     result = arca.run(f"file://{git_dir}", "master", task)
-    assert result.success
-    assert result.result == "Some other string"
+    assert result.output == "Some other string"
 
     # in the other branch there's still the original
     result = arca.run(f"file://{git_dir}", "new_branch", task)
-    assert result.success
-    assert result.result == "Some string"
+    assert result.output == "Some string"
 
     repo.branches.master.checkout()
 
@@ -114,12 +112,8 @@ def test_backends(backend, requirements_location, file_location):
     repo.index.commit("Added requirements, changed to version")
 
     result = arca.run(f"file://{git_dir}", "master", task)
-    try:
-        print(result.error)
-    except AttributeError:
-        pass
-    assert result.success
-    assert result.result == "1.11.4"
+
+    assert result.output == "1.11.4"
 
     with pytest.raises(ModuleNotFoundError):
         import django
@@ -132,24 +126,22 @@ def test_backends(backend, requirements_location, file_location):
     repo.index.commit("Updated requirements")
 
     result = arca.run(f"file://{git_dir}", "master", task)
-    assert result.success
-    assert result.result == "1.11.5"
+    assert result.output == "1.11.5"
 
     django_task = Task(
         "django.get_version",
         imports=["django"]
     )
 
     result = arca.run(f"file://{git_dir}", "master", django_task)
-    assert result.success
-    assert result.result == "1.11.5"
+    assert result.output == "1.11.5"
 
     django_task_error = Task(
         "django.get_version",
     )
 
-    result = arca.run(f"file://{git_dir}", "master", django_task_error)
-    assert not result.success
+    with pytest.raises(BuildError):
+        arca.run(f"file://{git_dir}", "master", django_task_error)
 
 
 @pytest.mark.parametrize(