From 190749ae4008ae01dad5033f79a841268caacce5 Mon Sep 17 00:00:00 2001 From: Johann Bahl Date: Mon, 12 Feb 2024 14:11:52 +0100 Subject: [PATCH] friendlier error messages --- lib.nix | 2 +- src/backy/api.py | 2 +- src/backy/backup.py | 68 ++++++++++++++++++++++-------------- src/backy/main.py | 53 ++++++++++++++++++++++------ src/backy/tests/test_api.py | 18 ++++++++++ src/backy/tests/test_main.py | 4 +-- 6 files changed, 107 insertions(+), 40 deletions(-) diff --git a/lib.nix b/lib.nix index 56204c2a..27c994af 100644 --- a/lib.nix +++ b/lib.nix @@ -121,7 +121,7 @@ in devShells = { default = mkShellNoCC { - BACKY_CMD = "backy"; + BACKY_CMD = "${poetryEnv}/bin/backy"; packages = [ poetryEnv poetry diff --git a/src/backy/api.py b/src/backy/api.py index b6a7e673..159c332f 100644 --- a/src/backy/api.py +++ b/src/backy/api.py @@ -88,7 +88,7 @@ async def reconfigure( ) await site.start() self.log.info("added-site", site=site.name) - for bind_addr, site in self.sites.items(): + for bind_addr, site in list(self.sites.items()): if bind_addr in bind_addrs: continue await site.stop() diff --git a/src/backy/backup.py b/src/backy/backup.py index 46cf89b9..9b930844 100644 --- a/src/backy/backup.py +++ b/src/backy/backup.py @@ -796,7 +796,7 @@ def find(self, spec: str) -> Revision: # Syncing Revisions @locked(target=".backup", mode="exclusive") - async def push_metadata(self, peers, taskid: str): + async def push_metadata(self, peers, taskid: str) -> int: grouped = defaultdict(list) for r in self.clean_history: if r.pending_changes: @@ -805,16 +805,20 @@ async def push_metadata(self, peers, taskid: str): "push-start", changes=sum(len(l) for l in grouped.values()) ) async with APIClientManager(peers, taskid, self.log) as apis: - await asyncio.gather( + errors = await asyncio.gather( *[ self._push_metadata(apis[server], grouped[server]) for server in apis ] ) - self.log.info("push-end") + self.log.info("push-end", errors=sum(errors)) + return sum(errors) - async def _push_metadata(self, api: APIClient, revs: List[Revision]): + async def _push_metadata( + self, api: APIClient, revs: List[Revision] + ) -> bool: purge_required = False + error = False for r in revs: log = self.log.bind( server=r.server, @@ -835,10 +839,13 @@ async def _push_metadata(self, api: APIClient, revs: List[Revision]): purge_required = True except ClientResponseError: log.warning("push-client-error", exc_style="short") + error = True except ClientConnectionError: - log.info("push-connection-error", exc_style="short") + log.warning("push-connection-error", exc_style="short") + error = True except ClientError: - log.warning("push-error", exc_info=True) + log.exception("push-error") + error = True if purge_required: log = self.log.bind(server=api.server_name) @@ -847,13 +854,17 @@ async def _push_metadata(self, api: APIClient, revs: List[Revision]): await api.run_purge(self.name) except ClientResponseError: log.warning("push-purge-client-error", exc_style="short") + error = True except ClientConnectionError: - log.info("push-purge-connection-error", exc_style="short") + log.warning("push-purge-connection-error", exc_style="short") + error = True except ClientError: - log.warning("push-purge-error", exc_info=True) + log.error("push-purge-error") + error = True + return error @locked(target=".backup", mode="exclusive") - async def pull_metadata(self, peers: dict, taskid: str): + async def pull_metadata(self, peers: dict, taskid: str) -> int: async def remove_dead_peer(): for r in list(self.history): if r.server and r.server not in peers: @@ -863,21 +874,24 @@ async def remove_dead_peer(): server=r.server, ) r.remove(force=True) + return False self.log.info("pull-start") async with APIClientManager(peers, taskid, self.log) as apis: - await asyncio.gather( + errors = await asyncio.gather( remove_dead_peer(), *[self._pull_metadata(apis[server]) for server in apis], ) - self.log.info("pull-end") + self.log.info("pull-end", errors=sum(errors)) + return sum(errors) - async def _pull_metadata(self, api: APIClient): + async def _pull_metadata(self, api: APIClient) -> bool: + error = False log = self.log.bind(server=api.server_name) try: await api.touch_backup(self.name) remote_revs = await api.get_revs(self) - + log.debug("pull-found-revs", revs=len(remote_revs)) except ClientResponseError as e: if e.status in [ HTTPNotFound.status_code, @@ -886,29 +900,31 @@ async def _pull_metadata(self, api: APIClient): log.debug("pull-not-found") else: log.warning("pull-client-error", exc_style="short") + error = True remote_revs = [] except ClientConnectionError: - log.info("pull-connection-error", exc_style="short") - return + log.warning("pull-connection-error", exc_style="short") + return True except ClientError: - log.warning("pull-error", exc_info=True) + log.exception("pull-error") + error = True remote_revs = [] - log.debug( - "pull-found-revs", - revs=len(remote_revs), - ) - matching_uuids = { + local_uuids = { r.uuid for r in self.history if r.server == api.server_name } remote_uuids = {r.uuid for r in remote_revs} - for uuid in matching_uuids - remote_uuids: + for uuid in local_uuids - remote_uuids: log.warning("pull-removing-unknown-rev", rev_uuid=uuid) self.find_by_uuid(uuid).remove(force=True) for r in remote_revs: + if r.uuid in local_uuids: + if r.to_dict() == self.find_by_uuid(r.uuid).to_dict(): + continue + log.debug("pull-updating-rev", rev_uid=r.uuid) + else: + log.debug("pull-new-rev", rev_uid=r.uuid) r.write_info() - log.debug( - "pull-updated-rev", - rev_uid=r.uuid, - ) + + return error diff --git a/src/backy/main.py b/src/backy/main.py index 2c065f22..a8f27f33 100644 --- a/src/backy/main.py +++ b/src/backy/main.py @@ -147,30 +147,58 @@ def verify(self, revision): b = backy.backup.Backup(self.path, self.log) b.verify(revision) - def client(self, config, peer, url, token, apifunc, **kwargs): - async def run(): + def client(self, config, peer, url, token, apifunc, **kwargs) -> int: + async def run() -> int: + if peer and (url or token): + self.log.error( + "client-argparse-error", + _fmt_msg="--peer conflicts with --url and --token", + ) + return 1 + if bool(url) ^ bool(token): + self.log.error( + "client-argparse-error", + _fmt_msg="--url and --token require each other", + ) + return 1 if url and token: api = APIClient("", url, token, self.taskid, self.log) else: d = backy.daemon.BackyDaemon(config, self.log) d._read_config() if peer: + if peer not in d.peers: + self.log.error( + "client-peer-unknown", + _fmt_msg="The peer {peer} is not known. Select a known peer or specify --url and --token.\n" + "The following peers are known: {known}", + peer=peer, + known=", ".join(d.peers.keys()), + ) + return 1 api = APIClient.from_conf( peer, d.peers[peer], self.taskid, self.log ) else: + if "token" not in d.api_cli_default: + self.log.error( + "client-missing-defaults", + _fmt_msg="The config file is missing default parameters. Please specify --url and --token", + ) + return 1 api = APIClient.from_conf( "", d.api_cli_default, self.taskid, self.log ) async with CLIClient(api, self.log) as c: try: await getattr(c, apifunc)(**kwargs) - except ClientConnectionError as e: + except ClientConnectionError: c.log.error("connection-error", exc_style="banner") c.log.debug("connection-error", exc_info=True) - sys.exit(1) + return 1 + return 0 - asyncio.run(run()) + return asyncio.run(run()) def tags(self, action, autoremove, expect, revision, tags, force): tags = set(t.strip() for t in tags.split(",")) @@ -192,17 +220,19 @@ def expire(self): b.expire() b.warn_pending_changes() - def push(self, config): + def push(self, config) -> int: d = backy.daemon.BackyDaemon(config, self.log) d._read_config() b = backy.backup.Backup(self.path, self.log) - asyncio.run(b.push_metadata(d.peers, self.taskid)) + errors = asyncio.run(b.push_metadata(d.peers, self.taskid)) + return int(bool(errors)) - def pull(self, config): + def pull(self, config) -> int: d = backy.daemon.BackyDaemon(config, self.log) d._read_config() b = backy.backup.Backup(self.path, self.log) - asyncio.run(b.pull_metadata(d.peers, self.taskid)) + errors = asyncio.run(b.pull_metadata(d.peers, self.taskid)) + return int(bool(errors)) def setup_argparser(): @@ -562,7 +592,10 @@ def main(): try: log.debug("parsed", func=args.func, func_args=func_args) - func(**func_args) + ret = func(**func_args) + if isinstance(ret, int): + log.debug("return-code", code=ret) + sys.exit(ret) log.debug("successful") sys.exit(0) except Exception: diff --git a/src/backy/tests/test_api.py b/src/backy/tests/test_api.py index 600b25d0..608b20cb 100644 --- a/src/backy/tests/test_api.py +++ b/src/backy/tests/test_api.py @@ -208,6 +208,24 @@ async def test_simple_sync(daemons, log): assert new_rev1.trust == rev1.trust assert new_rev1.server == "server-1" + rev1.distrust() + rev1.tags = {"manual:new"} + rev1.write_info() + + await j0.pull_metadata() + b0.scan() + + assert [r.uuid for r in b0.history] == [rev0.uuid, rev1.uuid] + new_rev1 = b0.history[1] + assert new_rev1.backup == b0 + assert new_rev1.timestamp == rev1.timestamp + assert new_rev1.backend_type == "" + assert new_rev1.stats == rev1.stats + assert new_rev1.tags == rev1.tags + assert new_rev1.orig_tags == rev1.tags + assert new_rev1.trust == rev1.trust + assert new_rev1.server == "server-1" + new_rev1.remove() assert [r.uuid for r in b0.history] == [rev0.uuid, rev1.uuid] assert new_rev1.tags == set() diff --git a/src/backy/tests/test_main.py b/src/backy/tests/test_main.py index f749cf99..5d09c88a 100644 --- a/src/backy/tests/test_main.py +++ b/src/backy/tests/test_main.py @@ -283,7 +283,7 @@ def test_call_client( ... D command/parsed func='client' func_args={{'config': '...', 'peer': None, \ 'url': None, 'token': None{", "*bool(args)}{str(args)[1:-1]}, 'apifunc': '{action}'}} ... D daemon/read-config ... -... D command/successful \n\ +... D command/return-code code=0 """ ) == utils.log_data @@ -465,7 +465,7 @@ def do_raise(*args, **kw): ... E command/failed exception_class='builtins.RuntimeError' exception_msg='test' exception>\tTraceback (most recent call last): exception>\t File ".../src/backy/main.py", line ..., in main -exception>\t func(**func_args) +exception>\t ret = func(**func_args) exception>\t File ".../src/backy/tests/test_main.py", line ..., in do_raise exception>\t raise RuntimeError("test") exception>\tRuntimeError: test