Merge pull request #67 from flyingcircusio/fix-backup-rc

Fix return code for backy backup

changelog.d/20240607_123138_jb_fix_backup_rc.rst

@@ -0,0 +1,3 @@
+.. A new scriv changelog fragment.
+
+- Fixed the return code for `backy backup` which could have caused an infinite loop in the scheduler

src/backy/backup.py

@@ -322,7 +322,7 @@ def tags(
 
     @locked(target=".backup", mode="exclusive")
     @locked(target=".purge", mode="shared")
-    def backup(self, tags: set[str], force: bool = False) -> None:
+    def backup(self, tags: set[str], force: bool = False) -> bool:
         if not force:
             self.validate_tags(tags)
 
@@ -381,6 +381,7 @@ def backup(self, tags: set[str], force: bool = False) -> None:
                 self.log.warning("inconsistent")
                 revision.backend.verify()
                 break
+        return verified
 
     @locked(target=".backup", mode="exclusive")
     def distrust(self, revision: str) -> None:

src/backy/main.py

@@ -97,16 +97,18 @@ def status(self, yaml_: bool, revision: str) -> None:
                 f"[yellow]{pending_changes} pending change(s)[/] (Push changes with `backy push`)"
             )
 
-    def backup(self, tags: str, force: bool) -> None:
+    def backup(self, tags: str, force: bool) -> int:
         b = Backup(self.path, self.log)
         b._clean()
         try:
             tags_ = set(t.strip() for t in tags.split(","))
-            b.backup(tags_, force)
+            success = b.backup(tags_, force)
+            return int(not success)
         except IOError as e:
             if e.errno not in [errno.EDEADLK, errno.EAGAIN]:
                 raise
-            self.log.info("backup-already-running")
+            self.log.warning("backup-already-running")
+            return 1
         finally:
             b._clean()
 

src/backy/tests/test_main.py

@@ -2,6 +2,7 @@
 import os
 import pprint
 import sys
+from functools import partialmethod
 
 import pytest
 
@@ -110,9 +111,10 @@ def test_verbose_logging(capsys, argv):
     assert exit.value.code == 0
 
 
-def print_args(*args, **kw):
+def print_args(*args, return_value=None, **kw):
     print(args)
     pprint.pprint(kw)
+    return return_value
 
 
 async def async_print_args(*args, **kw):
@@ -148,7 +150,8 @@ def test_call_status(capsys, backup, argv, monkeypatch):
     )
 
 
-def test_call_backup(tmp_path, capsys, argv, monkeypatch):
+@pytest.mark.parametrize("success", [False, True])
+def test_call_backup(success, tmp_path, capsys, argv, monkeypatch):
     os.makedirs(tmp_path / "backy")
     os.chdir(tmp_path / "backy")
 
@@ -170,7 +173,11 @@ def test_call_backup(tmp_path, capsys, argv, monkeypatch):
             )
         )
 
-    monkeypatch.setattr(backy.backup.Backup, "backup", print_args)
+    monkeypatch.setattr(
+        backy.backup.Backup,
+        "backup",
+        partialmethod(print_args, return_value=success),
+    )
     argv.extend(["-v", "backup", "manual:test"])
     utils.log_data = ""
     with pytest.raises(SystemExit) as exit:
@@ -187,16 +194,16 @@ def test_call_backup(tmp_path, capsys, argv, monkeypatch):
     )
     assert (
         Ellipsis(
-            """\
+            f"""\
 ... D command/invoked                     args='... -v backup manual:test'
-... D command/parsed                      func='backup' func_args={'force': False, 'tags': 'manual:test'}
+... D command/parsed                      func='backup' func_args={{'force': False, 'tags': 'manual:test'}}
 ... D quarantine/scan                     entries=0
-... D command/successful                  \n\
+... D command/return-code                 code={int(not success)}
 """
         )
         == utils.log_data
     )
-    assert exit.value.code == 0
+    assert exit.value.code == int(not success)
 
 
 def test_call_find(capsys, backup, argv, monkeypatch):