From 79dc6a50ba8f244f9d90a83d3e831cee3b3bb6b0 Mon Sep 17 00:00:00 2001 From: Andrew Olsen Date: Wed, 15 Apr 2020 11:21:18 +1200 Subject: [PATCH 1/3] Add exit codes that indicate what went wrong. --- sno/branch.py | 3 +- sno/checkout.py | 7 ++-- sno/cli.py | 4 +++ sno/commit.py | 14 ++++---- sno/context.py | 16 +++++---- sno/core.py | 6 ++-- sno/diff.py | 21 ++++++++---- sno/exceptions.py | 69 +++++++++++++++++++++++++++++++++++++++ sno/fsck.py | 8 +++-- sno/init.py | 40 ++++++++++++++++------- sno/pull.py | 7 ++-- sno/query.py | 7 ++-- sno/working_copy.py | 4 +-- tests/test_branch.py | 6 ++-- tests/test_commit.py | 12 ++++--- tests/test_diff.py | 7 ++-- tests/test_init.py | 26 +++++++++------ tests/test_status.py | 6 ++-- tests/test_workingcopy.py | 13 ++++---- 19 files changed, 197 insertions(+), 79 deletions(-) create mode 100644 sno/exceptions.py diff --git a/sno/branch.py b/sno/branch.py index e0e80289b..208584165 100644 --- a/sno/branch.py +++ b/sno/branch.py @@ -5,6 +5,7 @@ import pygit2 from .cli_util import do_json_option +from .exceptions import InvalidOperation from .exec import execvp @@ -33,7 +34,7 @@ def branch(ctx, do_json, args): if sargs & {"-d", "--delete", "-D"}: branch = repo.head.shorthand if branch in sargs: - raise click.ClickException( + raise InvalidOperation( f"Cannot delete the branch '{branch}' which you are currently on." ) diff --git a/sno/checkout.py b/sno/checkout.py index f6fab54fd..2415e58e5 100644 --- a/sno/checkout.py +++ b/sno/checkout.py @@ -4,6 +4,7 @@ import click import pygit2 +from .exceptions import InvalidOperation, NotFound, NO_WORKING_COPY from .structure import RepositoryStructure from .working_copy import WorkingCopy @@ -66,7 +67,7 @@ def checkout(ctx, branch, fmt, force, path, datasets, refish): wc = repo_structure.working_copy if wc: if path is not None: - raise click.ClickException( + raise InvalidOperation( f"This repository already has a working copy at: {wc.path}", ) @@ -221,7 +222,7 @@ def restore(ctx, source, pathspec): repo_structure = RepositoryStructure(repo) working_copy = repo_structure.working_copy if not working_copy: - raise click.ClickException("You don't have a working copy") + raise NotFound("You don't have a working copy", exit_code=NO_WORKING_COPY) head_commit = repo.head.peel(pygit2.Commit) @@ -246,7 +247,7 @@ def workingcopy_set_path(ctx, new): repo_cfg = repo.config if "sno.workingcopy.path" not in repo_cfg: - raise click.ClickException("No working copy? Try `sno checkout`") + raise NotFound("No working copy? Try `sno checkout`", exit_code=NO_WORKING_COPY) new = Path(new) # TODO: This doesn't seem to do anything? diff --git a/sno/cli.py b/sno/cli.py index fe2d6f92e..34e173ee2 100755 --- a/sno/cli.py +++ b/sno/cli.py @@ -13,6 +13,7 @@ clone, commit, diff, + exceptions, init, fsck, merge, @@ -25,6 +26,9 @@ from .exec import execvp +click.exceptions.UsageError.exit_code = exceptions.INVALID_ARGUMENT + + def print_version(ctx, param, value): if not value or ctx.resilient_parsing: return diff --git a/sno/commit.py b/sno/commit.py index 67f15d0dd..954848382 100644 --- a/sno/commit.py +++ b/sno/commit.py @@ -13,6 +13,7 @@ from . import is_windows from .core import check_git_user from .diff import Diff +from .exceptions import NotFound, SubprocessError, NO_CHANGES, NO_DATA, NO_WORKING_COPY from .status import ( get_branch_status_message, get_diff_status_message, @@ -64,8 +65,9 @@ def commit(ctx, message, message_file, allow_empty, do_json): repo = ctx.obj.repo if repo.is_empty: - raise click.UsageError( - 'Empty repository.\n (use "sno import" to add some data)' + raise NotFound( + 'Empty repository.\n (use "sno import" to add some data)', + exit_code=NO_DATA, ) check_git_user(repo) @@ -75,7 +77,7 @@ def commit(ctx, message, message_file, allow_empty, do_json): working_copy = WorkingCopy.open(repo) if not working_copy: - raise click.UsageError("No working copy, use 'checkout'") + raise NotFound("No working copy, use 'checkout'", exit_code=NO_WORKING_COPY) working_copy.assert_db_tree_match(tree) @@ -88,7 +90,7 @@ def commit(ctx, message, message_file, allow_empty, do_json): wc_changes[dataset.path] = diff.counts(dataset) if not wcdiff and not allow_empty: - raise click.ClickException("No changes to commit") + raise NotFound("No changes to commit", exit_code=NO_CHANGES) if message_file: commit_msg = message_file.read().strip() @@ -98,7 +100,7 @@ def commit(ctx, message, message_file, allow_empty, do_json): commit_msg = get_commit_message(repo, wc_changes, quiet=do_json) if not commit_msg: - raise click.Abort() + raise click.UsageError("No commit message") rs.commit(wcdiff, commit_msg, allow_empty=allow_empty) @@ -152,7 +154,7 @@ def get_commit_message(repo, wc_changes, quiet=False): try: subprocess.check_call(editor_cmd, shell=True) except subprocess.CalledProcessError as e: - raise click.ClickException( + raise SubprocessError( f"There was a problem with the editor '{editor}': {e}" ) from e diff --git a/sno/context.py b/sno/context.py index bf648d8c3..e33eab322 100644 --- a/sno/context.py +++ b/sno/context.py @@ -1,8 +1,9 @@ from pathlib import Path -import click import pygit2 +from .exceptions import NotFound, NO_REPOSITORY + class Context(object): DEFAULT_REPO_PATH = Path() @@ -41,11 +42,12 @@ def repo(self): if not self._repo or not self._repo.is_bare: if self.has_repo_path: - raise click.BadParameter( - "Not an existing repository", param_hint="--repo" - ) + message = "Not an existing repository" + param_hint = "repo" else: - raise click.UsageError( - "Current directory is not an existing repository" - ) + message = "Current directory is not an existing repository" + param_hint = None + + raise NotFound(message, exit_code=NO_REPOSITORY, param_hint=param_hint) + return self._repo diff --git a/sno/core.py b/sno/core.py index 9c43294f5..6cdced44f 100644 --- a/sno/core.py +++ b/sno/core.py @@ -1,7 +1,5 @@ -import os - import pygit2 -from click import ClickException +from .exceptions import NotFound, NO_USER def walk_tree(top, path="", topdown=True): @@ -99,4 +97,4 @@ def check_git_user(repo=None): msg.append("\n(sno uses the same credentials and configuration as git)") - raise ClickException("\n".join(msg)) + raise NotFound("\n".join(msg), exit_code=NO_USER) diff --git a/sno/diff.py b/sno/diff.py index 87184e2b3..ec2fd13ce 100644 --- a/sno/diff.py +++ b/sno/diff.py @@ -15,6 +15,13 @@ from . import gpkg from .cli_util import MutexOption +from .exceptions import ( + InvalidOperation, + NotYetImplemented, + NotFound, + NO_WORKING_COPY, + UNCATEGORIZED_ERROR, +) L = logging.getLogger("sno.diff") @@ -456,7 +463,9 @@ def diff(ctx, output_format, output_path, exit_code, args): L.debug("commit_target=working-copy") working_copy = WorkingCopy.open(repo) if not working_copy: - raise click.UsageError("No working copy, use 'checkout'") + raise NotFound( + "No working copy, use 'checkout'", exit_code=NO_WORKING_COPY + ) working_copy.assert_db_tree_match(commit_head.peel(pygit2.Tree)) @@ -469,12 +478,12 @@ def diff(ctx, output_format, output_path, exit_code, args): if not merge_base: # there is no relation between the commits - raise click.ClickException( + raise InvalidOperation( f"Commits {commit_base.id} and {c_target.id} aren't related." ) elif merge_base not in (commit_base.id, c_target.id): # this needs a 3-way diff and we don't support them yet - raise click.ClickException(f"Sorry, 3-way diffs aren't supported yet.") + raise NotYetImplemented(f"Sorry, 3-way diffs aren't supported yet.") base_rs = RepositoryStructure(repo, commit_base) all_datasets = {ds.path for ds in base_rs} @@ -537,14 +546,14 @@ def diff(ctx, output_format, output_path, exit_code, args): w(dataset, diff[dataset]) except click.ClickException as e: L.debug("Caught ClickException: %s", e) - if exit_code: - e.exit_code = 2 + if exit_code and e.exit_code == 1: + e.exit_code = UNCATEGORIZED_ERROR raise except Exception as e: L.debug("Caught non-ClickException: %s", e) if exit_code: click.secho(f"Error: {e}", fg="red", file=sys.stderr) - raise SystemExit(2) from e + raise SystemExit(UNCATEGORIZED_ERROR) from e else: raise else: diff --git a/sno/exceptions.py b/sno/exceptions.py new file mode 100644 index 000000000..87b9b5489 --- /dev/null +++ b/sno/exceptions.py @@ -0,0 +1,69 @@ +import click + +# Exit codes +INVALID_ARGUMENT = 10 + +INVALID_OPERATION = 20 + +NOT_YET_IMPLEMENTED = 30 + +NOT_FOUND = 40 +NO_REPOSITORY = 41 +NO_DATA = 42 +NO_BRANCH = 43 +NO_CHANGES = 44 +NO_WORKING_COPY = 45 +NO_USER = 46 +NO_IMPORT_SOURCE = 47 +NO_TABLE = 48 + +SUBPROCESS_ERROR = 50 + +UNCATEGORIZED_ERROR = 99 + + +class BaseException(click.ClickException): + """ + A ClickException that can easily be constructed with any exit code, + and which can also optionally give a hint about which param lead to + the problem. + Providing a param hint or not can be done for any type of error - + an unparseable import path and an import path that points to a + corrupted database might both provide the same hint, but be + considered completely different types of errors. + """ + + exit_code = UNCATEGORIZED_ERROR + + def __init__(self, message, exit_code=None, param=None, param_hint=None): + super(BaseException, self).__init__(message) + + if exit_code is not None: + self.exit_code = exit_code + + self.param_hint = None + if param_hint is not None: + self.param_hint = param_hint + elif param is not None: + self.param_hint = param.get_error_hint(None) + + def format_message(self): + if self.param_hint is not None: + return f"Invalid value for {self.param_hint}: {self.message}" + return self.message + + +class InvalidOperation(BaseException): + exit_code = INVALID_OPERATION + + +class NotYetImplemented(BaseException): + exit_code = NOT_YET_IMPLEMENTED + + +class NotFound(BaseException): + exit_code = NOT_FOUND + + +class SubprocessError(BaseException): + exit_code = SUBPROCESS_ERROR diff --git a/sno/fsck.py b/sno/fsck.py index 744bd39dc..c2c8656b1 100644 --- a/sno/fsck.py +++ b/sno/fsck.py @@ -5,7 +5,8 @@ import pygit2 from osgeo import gdal -from . import core, gpkg +from . import gpkg +from .exceptions import NotFound, NO_WORKING_COPY from .structure import RepositoryStructure @@ -87,8 +88,9 @@ def fsck(ctx, reset_datasets, fsck_args): working_copy_path = repo.config["sno.workingcopy.path"] if not os.path.isfile(working_copy_path): - raise click.ClickException( - click.style(f"Working copy missing: {working_copy_path}", fg="red") + raise NotFound( + click.style(f"Working copy missing: {working_copy_path}", fg="red"), + exit_code=NO_WORKING_COPY, ) working_copy = rs.working_copy diff --git a/sno/init.py b/sno/init.py index 5a9851bf2..3e3dade89 100644 --- a/sno/init.py +++ b/sno/init.py @@ -14,6 +14,13 @@ from . import gpkg, checkout, structure from .core import check_git_user from .cli_util import do_json_option +from .exceptions import ( + InvalidOperation, + NotFound, + INVALID_ARGUMENT, + NO_IMPORT_SOURCE, + NO_TABLE, +) @click.command("import-gpkg", hidden=True) @@ -88,7 +95,8 @@ def convert(self, value, param, ctx): prefix = prefix.upper() if prefix not in self.prefixes: self.fail( - f'invalid prefix: "{prefix}" (choose from {", ".join(self.prefixes)})' + f'invalid prefix: "{prefix}" (choose from {", ".join(self.prefixes)})', + exit_code=INVALID_ARGUMENT, ) # resolve GPKG:~/foo.gpkg and GPKG:~me/foo.gpkg @@ -99,6 +107,9 @@ def convert(self, value, param, ctx): path = super().convert(value, param, ctx) return (prefix, path, suffix) + def fail(self, message, param=None, ctx=None, exit_code=NO_IMPORT_SOURCE): + raise NotFound(message, exit_code=exit_code) + class ImportGPKG: """ GeoPackage Import Source """ @@ -108,7 +119,10 @@ class ImportGPKG: def __init__(self, source, table=None): self.source = source self.table = table - self.db = gpkg.db(self.source) + try: + self.db = gpkg.db(self.source) + except Exception: + raise RuntimeError("aaaaaaargh") def __str__(self): s = f"GeoPackage: {self.source}" @@ -124,8 +138,9 @@ def check(self): if not dbcur.execute(sql).fetchone(): raise ValueError("gpkg_contents table doesn't exist") except (ValueError, apsw.SQLError) as e: - raise ValueError( - f"'{self.source}' doesn't appear to be a valid GeoPackage" + raise NotFound( + f"'{self.source}' doesn't appear to be a valid GeoPackage", + exit_code=NO_IMPORT_SOURCE, ) from e if self.table: @@ -136,8 +151,9 @@ def check(self): table_name=? AND data_type IN ('features', 'attributes', 'aspatial');""" if not dbcur.execute(sql, (self.table,)).fetchone(): - raise ValueError( - f"Feature/Attributes table '{self.table}' not found in gpkg_contents" + raise NotFound( + f"Feature/Attributes table '{self.table}' not found in gpkg_contents", + exit_code=NO_TABLE, ) def get_table_list(self): @@ -333,8 +349,9 @@ def import_table(ctx, source, directory, do_list, do_json, version, method): try: source_loader.check() - except ValueError as e: - raise click.BadParameter(str(e), param_hint="source") from e + except NotFound as e: + e.param_hint = "source" + raise if do_list: source_loader.print_table_list(do_json=do_json) @@ -408,8 +425,9 @@ def init(ctx, import_from, do_checkout, directory): try: source_loader.check() - except ValueError as e: - raise click.BadParameter(str(e), param_hint="import_from") from e + except NotFound as e: + e.param_hint = "import_from" + raise if not import_table: import_table = source_loader.prompt_for_table("Select a table to import") @@ -424,7 +442,7 @@ def init(ctx, import_from, do_checkout, directory): repo_path = Path(directory).resolve() if any(repo_path.iterdir()): - raise click.BadParameter(f'"{repo_path}" isn\'t empty', param_hint="directory") + raise InvalidOperation(f'"{repo_path}" isn\'t empty', param_hint="directory") # Create the repository repo = pygit2.init_repository(str(repo_path), bare=True) diff --git a/sno/pull.py b/sno/pull.py index 919a0c9a6..a0cd2c5f2 100644 --- a/sno/pull.py +++ b/sno/pull.py @@ -1,7 +1,7 @@ import click -import pygit2 from . import merge +from .exceptions import NotFound, NO_BRANCH @click.command() @@ -39,11 +39,12 @@ def pull(ctx, ff, ff_only, repository, refspecs): if repository is None: # matches git-pull behaviour if repo.head_is_detached: - raise click.UsageError( + raise NotFound( ( "You are not currently on a branch. " "Please specify which branch you want to merge with." - ) + ), + exit_code=NO_BRANCH, ) # git-fetch: diff --git a/sno/query.py b/sno/query.py index ebc09f1e4..7b7d65024 100644 --- a/sno/query.py +++ b/sno/query.py @@ -1,17 +1,16 @@ import datetime import json import logging -import os import re import sys import time import types import click -import pygit2 from osgeo import ogr from . import structure +from .exceptions import NotFound L = logging.getLogger("sno.query") @@ -62,9 +61,7 @@ def query(ctx, path, command, params): try: dataset.get_spatial_index(dataset.name) except OSError: - raise click.ClickException( - "No spatial index found. Run `sno query {path} index`" - ) + raise NotFound("No spatial index found. Run `sno query {path} index`") if command == "get": USAGE = "get PK" diff --git a/sno/working_copy.py b/sno/working_copy.py index 8af3a6523..273f6999f 100644 --- a/sno/working_copy.py +++ b/sno/working_copy.py @@ -7,11 +7,11 @@ from datetime import datetime from pathlib import Path -import click import pygit2 from osgeo import gdal from . import gpkg, diff +from .exceptions import InvalidOperation L = logging.getLogger("sno.working_copy") @@ -763,7 +763,7 @@ def reset( dbcur.execute(f"SELECT COUNT(*) FROM {self.TRACKING_TABLE};") is_dirty = dbcur.fetchone()[0] if is_dirty and not force: - raise click.ClickException( + raise InvalidOperation( "You have uncommitted changes in your working copy. Commit or use --force to discard." ) diff --git a/tests/test_branch.py b/tests/test_branch.py index a3eea60f1..fadf487e8 100644 --- a/tests/test_branch.py +++ b/tests/test_branch.py @@ -3,6 +3,8 @@ import pytest +from sno.exceptions import NO_REPOSITORY + H = pytest.helpers.helpers() @@ -146,14 +148,14 @@ def test_branches_empty(tmp_path, cli_runner, chdir): def test_branches_none(tmp_path, cli_runner, chdir): with chdir(tmp_path): r = cli_runner.invoke(["branch"]) - assert r.exit_code == 2, r + assert r.exit_code == NO_REPOSITORY, r assert ( r.stdout.splitlines()[-1] == "Error: Current directory is not an existing repository" ) r = cli_runner.invoke(["branch", "--json"]) - assert r.exit_code == 2, r + assert r.exit_code == NO_REPOSITORY, r assert ( r.stdout.splitlines()[-1] == "Error: Current directory is not an existing repository" diff --git a/tests/test_commit.py b/tests/test_commit.py index e953152e6..54edfa62a 100644 --- a/tests/test_commit.py +++ b/tests/test_commit.py @@ -13,6 +13,8 @@ from sno.structure import RepositoryStructure from sno.working_copy import WorkingCopy +from sno.exceptions import INVALID_ARGUMENT, NO_CHANGES, NO_DATA, NO_REPOSITORY + H = pytest.helpers.helpers() @@ -75,7 +77,7 @@ def test_commit(archive, layer, data_working_copy, geopackage, cli_runner, reque with data_working_copy(archive) as (repo_dir, wc_path): # empty r = cli_runner.invoke(["commit", "-m", "test-commit-empty"]) - assert r.exit_code == 1, r + assert r.exit_code == NO_CHANGES, r assert r.stdout.splitlines() == ["Error: No changes to commit"] # empty @@ -182,7 +184,7 @@ def last_message(): # E: empty r = cli_runner.invoke(["commit", "--allow-empty", "-m", ""]) - assert r.exit_code == 1, r + assert r.exit_code == INVALID_ARGUMENT, r # file f_commit_message = str(tmp_path / "commit-message.txt") @@ -198,7 +200,7 @@ def last_message(): r = cli_runner.invoke( ["commit", "--allow-empty", "-F", f_commit_message, "-m", "foo"] ) - assert r.exit_code == 2, r + assert r.exit_code == INVALID_ARGUMENT, r assert "exclusive" in r.stdout # multiple @@ -282,7 +284,7 @@ def test_empty(tmp_path, cli_runner, chdir): assert r.exit_code == 0, r with chdir(repo_path): r = cli_runner.invoke(["commit", "--allow-empty"]) - assert r.exit_code == 2, r + assert r.exit_code == NO_DATA, r assert "Empty repository" in r.stdout # empty dir @@ -290,5 +292,5 @@ def test_empty(tmp_path, cli_runner, chdir): empty_path.mkdir() with chdir(empty_path): r = cli_runner.invoke(["commit", "--allow-empty"]) - assert r.exit_code == 2, r + assert r.exit_code == NO_REPOSITORY, r assert "not an existing repository" in r.stdout diff --git a/tests/test_diff.py b/tests/test_diff.py index 8707bea4a..2e5fef51a 100644 --- a/tests/test_diff.py +++ b/tests/test_diff.py @@ -7,6 +7,7 @@ import pygit2 from sno.diff import Diff +from sno.exceptions import NOT_YET_IMPLEMENTED H = pytest.helpers.helpers() @@ -1645,15 +1646,15 @@ def test_diff_3way(data_working_copy, geopackage, cli_runner, insert, request): # changes <> master (commit <> commit) diff should fail r = cli_runner.invoke(["diff", "--quiet", f"{m_commit_id}..{b_commit_id}"]) - assert r.exit_code == 2, r + assert r.exit_code == NOT_YET_IMPLEMENTED, r assert "3-way diffs aren't supported" in r.stdout # same the other way around r = cli_runner.invoke(["diff", "--quiet", f"{b_commit_id}..{m_commit_id}"]) - assert r.exit_code == 2, r + assert r.exit_code == NOT_YET_IMPLEMENTED, r assert "3-way diffs aren't supported" in r.stdout # diff against working copy should fail too r = cli_runner.invoke(["diff", "--quiet", b_commit_id]) - assert r.exit_code == 2, r + assert r.exit_code == NOT_YET_IMPLEMENTED, r assert "3-way diffs aren't supported" in r.stdout diff --git a/tests/test_init.py b/tests/test_init.py index de0b2bba4..55d17bd7b 100644 --- a/tests/test_init.py +++ b/tests/test_init.py @@ -6,6 +6,12 @@ import pygit2 from sno.working_copy import WorkingCopy +from sno.exceptions import ( + INVALID_ARGUMENT, + INVALID_OPERATION, + NO_IMPORT_SOURCE, + NO_TABLE, +) H = pytest.helpers.helpers() @@ -79,7 +85,7 @@ def test_import_geopackage(archive, gpkg, table, data_archive, tmp_path, cli_run r = cli_runner.invoke( [f"--repo={repo_path}", "import-gpkg", data / gpkg, table] ) - assert r.exit_code == 2, r + assert r.exit_code == INVALID_OPERATION, r assert re.search( r"^Error: Invalid value for directory: \".*\" isn't empty", r.stdout, @@ -99,7 +105,7 @@ def test_import_geopackage_errors(data_archive, tmp_path, cli_runner): "some-layer-that-doesn't-exist", ] ) - assert r.exit_code == 2, r + assert r.exit_code == NO_TABLE, r assert ( "Feature/Attributes table 'some-layer-that-doesn't-exist' not found in gpkg_contents" in r.stdout @@ -117,7 +123,7 @@ def test_import_geopackage_errors(data_archive, tmp_path, cli_runner): r = cli_runner.invoke( [f"--repo={repo_path}", "import-gpkg", tmp_path / "a.gpkg", "mytable"] ) - assert r.exit_code == 2, r + assert r.exit_code == NO_IMPORT_SOURCE, r assert "a.gpkg' doesn't appear to be a valid GeoPackage" in r.stdout @@ -238,15 +244,15 @@ def test_init_import_errors(data_archive, tmp_path, cli_runner): repo_path.mkdir() r = cli_runner.invoke(["init", "--import", f"fred:thingz"]) - assert r.exit_code == 2, r + assert r.exit_code == INVALID_ARGUMENT, r assert 'invalid prefix: "FRED" (choose from GPKG)' in r.stdout r = cli_runner.invoke(["init", "--import", f"gpkg:thingz.gpkg"]) - assert r.exit_code == 2, r + assert r.exit_code == NO_IMPORT_SOURCE, r assert 'File "thingz.gpkg" does not exist.' in r.stdout r = cli_runner.invoke(["init", "--import", f"gpkg:{data/gpkg}:no-existey"]) - assert r.exit_code == 2, r + assert r.exit_code == NO_TABLE, r assert ( "Feature/Attributes table 'no-existey' not found in gpkg_contents" in r.stdout @@ -257,7 +263,7 @@ def test_init_import_errors(data_archive, tmp_path, cli_runner): r = cli_runner.invoke( ["init", "--import", f"gpkg:{data/gpkg}:{table}", repo_path] ) - assert r.exit_code == 2, r + assert r.exit_code == INVALID_OPERATION, r assert "isn't empty" in r.stdout # import @@ -281,7 +287,7 @@ def test_init_import_errors(data_archive, tmp_path, cli_runner): r = cli_runner.invoke( ["init", "--import", f"gpkg:{data / gpkg}:{table}", repo_path] ) - assert r.exit_code == 2, r + assert r.exit_code == INVALID_OPERATION, r assert "isn't empty" in r.stdout @@ -314,13 +320,13 @@ def test_init_empty(tmp_path, cli_runner, chdir): repo_path.mkdir() (repo_path / "a.file").touch() r = cli_runner.invoke(["init", repo_path]) - assert r.exit_code == 2, r + assert r.exit_code == INVALID_OPERATION, r assert not (repo_path / "HEAD").exists() # current dir isn't empty with chdir(repo_path): r = cli_runner.invoke(["init"]) - assert r.exit_code == 2, r + assert r.exit_code == INVALID_OPERATION, r assert not (repo_path / "HEAD").exists() diff --git a/tests/test_status.py b/tests/test_status.py index 27a7acc4f..f3fcda677 100644 --- a/tests/test_status.py +++ b/tests/test_status.py @@ -3,6 +3,8 @@ import pytest +from sno.exceptions import NO_REPOSITORY + H = pytest.helpers.helpers() @@ -245,14 +247,14 @@ def test_status_empty(tmp_path, cli_runner, chdir): def test_status_none(tmp_path, cli_runner, chdir): with chdir(tmp_path): r = cli_runner.invoke(["status"]) - assert r.exit_code == 2, r + assert r.exit_code == NO_REPOSITORY, r assert ( r.stdout.splitlines()[-1] == "Error: Current directory is not an existing repository" ) r = cli_runner.invoke(["status", "--json"]) - assert r.exit_code == 2, r + assert r.exit_code == NO_REPOSITORY, r assert ( r.stdout.splitlines()[-1] == "Error: Current directory is not an existing repository" diff --git a/tests/test_workingcopy.py b/tests/test_workingcopy.py index b0af010e1..ad65294da 100644 --- a/tests/test_workingcopy.py +++ b/tests/test_workingcopy.py @@ -8,6 +8,7 @@ import sno.checkout from sno.working_copy import WorkingCopy +from sno.exceptions import INVALID_ARGUMENT, INVALID_OPERATION H = pytest.helpers.helpers() @@ -155,7 +156,7 @@ def test_checkout_branch(data_working_copy, geopackage, cli_runner, tmp_path): # creating a new branch with existing name errors r = cli_runner.invoke(["checkout", "-b", "master"]) - assert r.exit_code == 2, r + assert r.exit_code == INVALID_ARGUMENT, r assert r.stdout.splitlines()[-1].endswith( "A branch named 'master' already exists." ) @@ -268,7 +269,7 @@ def test_switch_branch(data_working_copy, geopackage, cli_runner, tmp_path): assert db.changes() == 1 r = cli_runner.invoke(["switch", "foo"]) - assert r.exit_code == 1, r + assert r.exit_code == INVALID_OPERATION, r assert "Error: You have uncommitted changes in your working copy." in r.stdout r = cli_runner.invoke(["switch", "foo", "--discard-changes"]) @@ -384,7 +385,7 @@ def test_working_copy_reset( # this should error r = cli_runner.invoke(["checkout", "HEAD"]) - assert r.exit_code == 1, r + assert r.exit_code == INVALID_OPERATION, r cur.execute("""SELECT COUNT(*) FROM ".sno-track";""") change_count = cur.fetchone()[0] @@ -468,7 +469,7 @@ def test_workingcopy_set_path(data_working_copy, cli_runner, tmp_path): repo = pygit2.Repository(str(repo_path)) r = cli_runner.invoke(["workingcopy-set-path", "/thingz.gpkg"]) - assert r.exit_code == 2, r + assert r.exit_code == INVALID_ARGUMENT, r # relative path 1 new_path = Path("new-thingz.gpkg") @@ -658,14 +659,14 @@ def test_delete_branch(data_working_copy, cli_runner): with data_working_copy("points") as (repo_path, wc): # prevent deleting the current branch r = cli_runner.invoke(["branch", "-d", "master"]) - assert r.exit_code == 1, r + assert r.exit_code == INVALID_OPERATION, r assert "Cannot delete" in r.stdout r = cli_runner.invoke(["checkout", "-b", "test"]) assert r.exit_code == 0, r r = cli_runner.invoke(["branch", "-d", "test"]) - assert r.exit_code == 1, r + assert r.exit_code == INVALID_OPERATION, r r = cli_runner.invoke(["checkout", "master"]) assert r.exit_code == 0, r From 703ede81120688a760de6567c8c9adaf2da850fd Mon Sep 17 00:00:00 2001 From: Andrew Olsen Date: Thu, 16 Apr 2020 10:37:35 +1200 Subject: [PATCH 2/3] Change numbers - usage error is 2, uncagetorized is now 11. --- sno/cli.py | 4 ---- sno/commit.py | 3 ++- sno/diff.py | 9 +++++++-- sno/exceptions.py | 36 +++++++++++++++++++++++++++++------- 4 files changed, 38 insertions(+), 14 deletions(-) diff --git a/sno/cli.py b/sno/cli.py index 34e173ee2..fe2d6f92e 100755 --- a/sno/cli.py +++ b/sno/cli.py @@ -13,7 +13,6 @@ clone, commit, diff, - exceptions, init, fsck, merge, @@ -26,9 +25,6 @@ from .exec import execvp -click.exceptions.UsageError.exit_code = exceptions.INVALID_ARGUMENT - - def print_version(ctx, param, value): if not value or ctx.resilient_parsing: return diff --git a/sno/commit.py b/sno/commit.py index 954848382..fe898b90d 100644 --- a/sno/commit.py +++ b/sno/commit.py @@ -155,7 +155,8 @@ def get_commit_message(repo, wc_changes, quiet=False): subprocess.check_call(editor_cmd, shell=True) except subprocess.CalledProcessError as e: raise SubprocessError( - f"There was a problem with the editor '{editor}': {e}" + f"There was a problem with the editor '{editor}': {e}", + called_process_error=e, ) from e with open(commit_editmsg, "rt", encoding="utf-8") as f: diff --git a/sno/diff.py b/sno/diff.py index ec2fd13ce..88e672081 100644 --- a/sno/diff.py +++ b/sno/diff.py @@ -20,6 +20,7 @@ NotYetImplemented, NotFound, NO_WORKING_COPY, + NO_COMMIT, UNCATEGORIZED_ERROR, ) @@ -442,14 +443,18 @@ def diff(ctx, output_format, output_path, exit_code, args): commit_target = repo.revparse_single(commit_parts[2] or "HEAD") L.debug("commit_target=%s", commit_target.id) except KeyError: - raise click.BadParameter("Invalid commit spec", param_hint="commit") + raise NotFound( + "Invalid commit spec", param_hint="commit", exit_code=NO_COMMIT + ) else: path_list.pop(0) else: try: commit_base = repo.revparse_single(commit_parts[0]) except KeyError: - raise click.BadParameter("Invalid commit spec", param_hint="commit") + raise NotFound( + "Invalid commit spec", param_hint="commit", exit_code=NO_COMMIT + ) else: path_list.pop(0) diff --git a/sno/exceptions.py b/sno/exceptions.py index 87b9b5489..c9857cfe8 100644 --- a/sno/exceptions.py +++ b/sno/exceptions.py @@ -1,7 +1,15 @@ import click # Exit codes -INVALID_ARGUMENT = 10 + +SUCCESS = 0 +SUCCESS_WITH_FLAG = 1 + +INVALID_ARGUMENT = 2 + +# We could use 1 for this, except in --exit-code mode. +# So we always use 11 for consistency. +UNCATEGORIZED_ERROR = 11 INVALID_OPERATION = 20 @@ -14,12 +22,12 @@ NO_CHANGES = 44 NO_WORKING_COPY = 45 NO_USER = 46 -NO_IMPORT_SOURCE = 47 -NO_TABLE = 48 - -SUBPROCESS_ERROR = 50 +NO_COMMIT = 47 +NO_IMPORT_SOURCE = 48 +NO_TABLE = 49 -UNCATEGORIZED_ERROR = 99 +SUBPROCESS_ERROR_FLAG = 128 +DEFAULT_SUBPROCESS_ERROR = 129 class BaseException(click.ClickException): @@ -66,4 +74,18 @@ class NotFound(BaseException): class SubprocessError(BaseException): - exit_code = SUBPROCESS_ERROR + exit_code = DEFAULT_SUBPROCESS_ERROR + + def __init__( + self, + message, + exit_code=None, + param=None, + param_hint=None, + called_process_error=None, + ): + super(SubprocessError, self).__init__( + message, exit_code=exit_code, param=param, param_hint=param_hint + ) + if called_process_error and not exit_code: + self.exit_code = SUBPROCESS_ERROR_FLAG + called_process_error.return_code From f2af66c547396ddfd9c9e88278475f30a72f91ac Mon Sep 17 00:00:00 2001 From: Andrew Olsen Date: Thu, 16 Apr 2020 11:36:26 +1200 Subject: [PATCH 3/3] Remove accidental test code --- sno/init.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/sno/init.py b/sno/init.py index 3e3dade89..dbf8542b8 100644 --- a/sno/init.py +++ b/sno/init.py @@ -119,10 +119,7 @@ class ImportGPKG: def __init__(self, source, table=None): self.source = source self.table = table - try: - self.db = gpkg.db(self.source) - except Exception: - raise RuntimeError("aaaaaaargh") + self.db = gpkg.db(self.source) def __str__(self): s = f"GeoPackage: {self.source}"