[Concept,28/29] patman: Clarify the 'series not in database' error path

Message ID 20260501110040.1874719-29-sjg@u-boot.org
State New
Headers
Series patman: Review-flow improvements and shared helpers |

Commit Message

Simon Glass May 1, 2026, 11 a.m. UTC
  From: Simon Glass <sjg@chromium.org>

Running 'patman series scan' on a branch that has not been added to
the database fails with:

    patman: ValueError: No matching series for id None version 1

The same shape of confusing error -- a database lookup with idnum
None being reported as raw SQL -- is reachable from 'series set-link',
'series get-link', 'series save-notes' and 'series show-notes' too,
because each of those subcommands also passes ser.idnum on without
checking that the series was actually found.

Pull the existing 'if not ser.idnum: raise ValueError(...)' pattern
out of the ten places it appears in cseries.py into a helper,
_ensure_in_db(), in cser_helper.py. The helper raises a single
canonical message that names the series and points at
'patman series add' as the fix:

    Series 'foo' not found in database; use 'patman series add' first

Use the helper in every existing site (so the message is uniform
across subcommands) and add it to the four sites that previously
let an unset idnum reach the database.

Update the two tests that asserted on the old per-site error wording
('No such series') to expect the unified message.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 tools/patman/cser_helper.py  | 20 ++++++++++++++++++++
 tools/patman/cseries.py      | 32 ++++++++++++++------------------
 tools/patman/test_cseries.py | 11 ++++++++---
 3 files changed, 42 insertions(+), 21 deletions(-)
  

Patch

diff --git a/tools/patman/cser_helper.py b/tools/patman/cser_helper.py
index 032d5218e9e..0ce59e8ecc0 100644
--- a/tools/patman/cser_helper.py
+++ b/tools/patman/cser_helper.py
@@ -731,6 +731,26 @@  class CseriesHelper:
         recs = self.get_ser_ver(series_id, version)
         return recs.idnum, recs.link
 
+    def _ensure_in_db(self, ser):
+        """Verify a series object came from the database.
+
+        Many subcommands look up a series by name, get a Series object back
+        with idnum unset when no row exists, then pass that idnum on to the
+        database where it is reported only as 'series_id NULL'. Raise a
+        clear ValueError up front so the caller knows to register the
+        series first.
+
+        Args:
+            ser (Series): Series object whose idnum must be set
+
+        Raises:
+            ValueError: if ser.idnum is None
+        """
+        if not ser.idnum:
+            raise ValueError(
+                f"Series '{ser.name}' not found in database; "
+                "use 'patman series add' first")
+
     def get_ser_ver(self, series_id, version):
         """Get the patchwork details for a series version
 
diff --git a/tools/patman/cseries.py b/tools/patman/cseries.py
index 443fe324c37..fb3f5a6c49a 100644
--- a/tools/patman/cseries.py
+++ b/tools/patman/cseries.py
@@ -126,8 +126,7 @@  class Cseries(cser_helper.CseriesHelper):
             dry_run (bool): True to do a dry run
         """
         ser = self._parse_series(series)
-        if not ser.idnum:
-            raise ValueError(f"Series '{ser.name}' not found in database")
+        self._ensure_in_db(ser)
 
         max_vers = self._series_max_version(ser.idnum)
         if max_vers < 2:
@@ -170,8 +169,7 @@  class Cseries(cser_helper.CseriesHelper):
             dry_run (bool): True to do a dry run
         """
         ser = self._parse_series(series_name)
-        if not ser.idnum:
-            raise ValueError(f"Series '{ser.name}' not found in database")
+        self._ensure_in_db(ser)
 
         max_vers = self._series_max_version(ser.idnum)
 
@@ -230,6 +228,7 @@  class Cseries(cser_helper.CseriesHelper):
                 link
         """
         ser, version = self._parse_series_and_version(series_name, version)
+        self._ensure_in_db(ser)
         self._ensure_version(ser, version)
 
         self._set_link(ser.idnum, ser.name, version, link, update_commit)
@@ -248,6 +247,7 @@  class Cseries(cser_helper.CseriesHelper):
             str: Patchwork link as a string, e.g. '12325'
         """
         ser, version = self._parse_series_and_version(series, version)
+        self._ensure_in_db(ser)
         self._ensure_version(ser, version)
         return self.db.ser_ver_get_link(ser.idnum, version)
 
@@ -825,8 +825,7 @@  class Cseries(cser_helper.CseriesHelper):
         """
         ser = self._parse_series(name)
         name = ser.name
-        if not ser.idnum:
-            raise ValueError(f"No such series '{name}'")
+        self._ensure_in_db(ser)
 
         self.db.ser_ver_remove(ser.idnum, None)
         if not dry_run:
@@ -851,8 +850,7 @@  class Cseries(cser_helper.CseriesHelper):
             dry_run (bool): True to do a dry run
         """
         old_ser, _ = self._parse_series_and_version(series, None)
-        if not old_ser.idnum:
-            raise ValueError(f"Series '{old_ser.name}' not found in database")
+        self._ensure_in_db(old_ser)
         if old_ser.name != series:
             raise ValueError(f"Invalid series name '{series}': "
                              'did you use the branch name?')
@@ -922,6 +920,7 @@  class Cseries(cser_helper.CseriesHelper):
 
         notes = tools.read_file(notes_file, binary=False).strip()
         ser, version = self._parse_series_and_version(series, None)
+        self._ensure_in_db(ser)
         svid = self.get_series_svid(ser.idnum, version)
         self.db.ser_ver_set_notes(svid, notes)
         self.commit()
@@ -934,6 +933,7 @@  class Cseries(cser_helper.CseriesHelper):
             series (str): Series name, or None for current branch
         """
         ser, _ = self._parse_series_and_version(series, None)
+        self._ensure_in_db(ser)
         all_notes = self.db.ser_ver_get_all_notes(ser.idnum)
         if not all_notes:
             tout.notice(f"No review notes for '{ser.name}'")
@@ -954,8 +954,7 @@  class Cseries(cser_helper.CseriesHelper):
                 the listed patch numbers (1-based).
         """
         ser, _ = self._parse_series_and_version(series, None)
-        if not ser.idnum:
-            raise ValueError(f"Series '{ser.name}' not found in database")
+        self._ensure_in_db(ser)
 
         with terminal.pager():
             self._show_info(ser, show_reviews)
@@ -1058,8 +1057,7 @@  class Cseries(cser_helper.CseriesHelper):
         if not ups:
             raise ValueError('Please specify the upstream name')
         ser, _ = self._parse_series_and_version(series, None)
-        if not ser.idnum:
-            raise ValueError(f"Series '{ser.name}' not found in database")
+        self._ensure_in_db(ser)
 
         self.db.series_set_upstream(ser.idnum, ups)
 
@@ -1093,6 +1091,7 @@  class Cseries(cser_helper.CseriesHelper):
             tout.info(f'{oper} {seq:3} {out}')
 
         name, ser, version, msg = self.prep_series(branch_name, end)
+        self._ensure_in_db(ser)
         svid = self.get_ser_ver(ser.idnum, version).idnum
         pcdict = self.get_pcommit_dict(svid)
 
@@ -1191,8 +1190,7 @@  class Cseries(cser_helper.CseriesHelper):
                 succeed
         """
         ser, version = self._parse_series_and_version(name, None)
-        if not ser.idnum:
-            raise ValueError(f"Series '{ser.name}' not found in database")
+        self._ensure_in_db(ser)
 
         ups = self.get_series_upstream(name)
         if ups:
@@ -1226,8 +1224,7 @@  class Cseries(cser_helper.CseriesHelper):
             series (str): Name of series to use, or None to use current branch
         """
         ser = self._parse_series(series, include_archived=True)
-        if not ser.idnum:
-            raise ValueError(f"Series '{ser.name}' not found in database")
+        self._ensure_in_db(ser)
 
         svlist = self.db.ser_ver_get_for_series(ser.idnum)
 
@@ -1275,8 +1272,7 @@  class Cseries(cser_helper.CseriesHelper):
             series (str): Name of series to use, or None to use current branch
         """
         ser = self._parse_series(series, include_archived=True)
-        if not ser.idnum:
-            raise ValueError(f"Series '{ser.name}' not found in database")
+        self._ensure_in_db(ser)
         self.db.series_set_archived(ser.idnum, False)
 
         svlist = self.db.ser_ver_get_for_series(ser.idnum)
diff --git a/tools/patman/test_cseries.py b/tools/patman/test_cseries.py
index c267b048fc5..7d6f99b3536 100644
--- a/tools/patman/test_cseries.py
+++ b/tools/patman/test_cseries.py
@@ -2384,7 +2384,10 @@  Tested-by: Mary Smith <msmith@wibble.com>   # yak
         with self.stage('remove non-existent series'):
             with self.assertRaises(ValueError) as exc:
                 cser.remove('first')
-            self.assertEqual("No such series 'first'", str(exc.exception))
+            self.assertEqual(
+                "Series 'first' not found in database; use "
+                "'patman series add' first",
+                str(exc.exception))
 
         with self.stage('add'):
             with terminal.capture() as (out, _):
@@ -2410,8 +2413,10 @@  Tested-by: Mary Smith <msmith@wibble.com>   # yak
             with terminal.capture() as (out, _):
                 self.run_args('series', '-s', 'first', 'rm', expect_ret=1,
                               pwork=True)
-            self.assertEqual("patman: ValueError: No such series 'first'",
-                             out.getvalue().strip())
+            self.assertEqual(
+                "patman: ValueError: Series 'first' not found in database;"
+                " use 'patman series add' first",
+                out.getvalue().strip())
 
         with self.stage('add'):
             with terminal.capture() as (out, _):