[PATCH b4 06/27] review: don't let archiving a series raise

[Christian Brauner <brauner@kernel.org>]

archive_series() builds a tarball and writes it into the data directory
without guarding any of it, so ENOSPC, a permission problem or a missing
object comes back to the caller as an exception.

Both callers archive a series that has just been thanked, the tracking
UI right after the SMTP handoff and queue delivery right after the
message goes out. The mail is gone by then. An exception unwinding
through the send path is reported as a failure to send, which tells the
maintainer to send a note that is already on the list, or it escapes the
screen callback and takes the rest of the TUI session with it.

Report the write failure as (False, detail) like every other failure
here. Nothing has been destroyed at that point so the archive can be
retried. Do the same for the Patchwork update, except that it comes
last. The local archive is done and can't be retried, so a Patchwork
hiccup is a warning and not a failed archive.

Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 src/b4/review/_review.py | 70 ++++++++++++++++++++++++++++--------------------
 1 file changed, 41 insertions(+), 29 deletions(-)

diff --git a/src/b4/review/_review.py b/src/b4/review/_review.py
index da20fc4..4c67387 100644
--- a/src/b4/review/_review.py
+++ b/src/b4/review/_review.py
@@ -809,6 +809,8 @@ def archive_series(
 
     Returns (success, detail): detail is the archive tarball path on
     success (empty for a database-only archive), or an error message.
+    Never raises: callers archive *after* a thank-you has gone out, and a
+    failure here must not be mistaken for a failure to deliver it.
     """
     # Imported here: the tarball machinery is only needed when archiving.
     # b4.review.tracking is re-imported alongside b4.ez because a local
@@ -833,36 +835,40 @@ def archive_series(
         if not first_patch:
             return False, 'No patch commits found in tracking data'
 
-        tio = io.BytesIO()
-        mnow = int(time.time())
-        with tarfile.open(fileobj=tio, mode='w:gz') as tfh:
-            # Add cover letter
-            ifh = io.BytesIO()
-            ifh.write(cover_text.encode())
-            b4.ez.write_to_tar(tfh, f'{change_id}/cover.txt', mnow, ifh)
-            ifh.close()
-            # Add tracking metadata
-            ifh = io.BytesIO()
-            ifh.write(make_review_magic_json(tracking).encode())
-            b4.ez.write_to_tar(tfh, f'{change_id}/tracking.js', mnow, ifh)
-            ifh.close()
-            # Add patches as mbox
-            patches = b4.git_range_to_patches(
-                topdir, f'{first_patch}~1', f'{review_branch}~1'
-            )
-            if patches:
+        try:
+            tio = io.BytesIO()
+            mnow = int(time.time())
+            with tarfile.open(fileobj=tio, mode='w:gz') as tfh:
+                # Add cover letter
                 ifh = io.BytesIO()
-                b4.save_git_am_mbox([patch[1] for patch in patches], ifh)
-                b4.ez.write_to_tar(tfh, f'{change_id}/patches.mbx', mnow, ifh)
+                ifh.write(cover_text.encode())
+                b4.ez.write_to_tar(tfh, f'{change_id}/cover.txt', mnow, ifh)
                 ifh.close()
+                # Add tracking metadata
+                ifh = io.BytesIO()
+                ifh.write(make_review_magic_json(tracking).encode())
+                b4.ez.write_to_tar(tfh, f'{change_id}/tracking.js', mnow, ifh)
+                ifh.close()
+                # Add patches as mbox
+                patches = b4.git_range_to_patches(
+                    topdir, f'{first_patch}~1', f'{review_branch}~1'
+                )
+                if patches:
+                    ifh = io.BytesIO()
+                    b4.save_git_am_mbox([patch[1] for patch in patches], ifh)
+                    b4.ez.write_to_tar(tfh, f'{change_id}/patches.mbx', mnow, ifh)
+                    ifh.close()
 
-        # Write archive to data directory
-        datadir = b4.get_data_dir()
-        archpath = os.path.join(datadir, 'review-archived')
-        os.makedirs(archpath, exist_ok=True)
-        tarpath = os.path.join(archpath, f'{change_id}.tar.gz')
-        with open(tarpath, mode='wb') as tout:
-            tout.write(tio.getvalue())
+            # Write archive to data directory
+            datadir = b4.get_data_dir()
+            archpath = os.path.join(datadir, 'review-archived')
+            os.makedirs(archpath, exist_ok=True)
+            tarpath = os.path.join(archpath, f'{change_id}.tar.gz')
+            with open(tarpath, mode='wb') as tout:
+                tout.write(tio.getvalue())
+        except Exception as ex:
+            # The branch is still intact, so this is safe to retry
+            return False, f'Could not write archive for {change_id}: {ex}'
 
         ok, err = delete_review_branch(topdir, review_branch, allow_switch=allow_switch)
         if not ok:
@@ -878,9 +884,15 @@ def archive_series(
     except Exception as ex:
         return False, f'DB error: {ex}'
 
-    # Mark as archived in Patchwork
+    # Mark as archived in Patchwork.  The local archive is already done and
+    # cannot be retried, so a Patchwork hiccup is a warning, not a failure.
     if pw_series_id:
-        pw_update_series_state(pw_series_id, 'accepted', archived=True)
+        try:
+            pw_update_series_state(pw_series_id, 'accepted', archived=True)
+        except Exception as ex:
+            logger.warning(
+                'Could not archive series %s in Patchwork: %s', change_id, ex
+            )
 
     return True, tarpath
 

-- 
2.53.0