[Concept,06/29] patman: Use patchwork.py for all HTTP access in review.py

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

Commit Message

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

review.py reaches into patchwork in two ways: via public Patchwork
methods (get_series, get_cover_comments, etc.) and, in a few
remaining places, by either constructing patchwork URLs itself or
calling Patchwork._request() directly. The direct paths are:

- fetch_mbox() builds 'series/<link>/mbox/' itself and downloads via
  aiohttp, so the apply_series chain takes 'pwork_url' (a bare string)
  rather than the Patchwork instance.
- _do_learn_voice() pokes pwork._request(client, 'projects/') to find
  the list_email for the configured project.

Add a get_series_mbox() method to Patchwork so the mbox URL is
expressed once, and a search_patches() public wrapper around the
existing 'patches/?project=...&q=...' subpath. Then switch review.py
over: fetch_mbox(), apply_series(), and apply_series_sync() now take
the Patchwork instance, _apply_and_check() passes ctx.pwork instead
of ctx.pwork.url, and _do_learn_voice() uses pwork.get_projects().

review.py no longer constructs patchwork URLs or calls the private
_request().

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

 tools/patman/patchwork.py | 46 +++++++++++++++++++++++++++++++++++++++
 tools/patman/review.py    | 42 +++++++++++++++--------------------
 2 files changed, 63 insertions(+), 25 deletions(-)
  

Patch

diff --git a/tools/patman/patchwork.py b/tools/patman/patchwork.py
index 5613cbb3383..46bacee50d2 100644
--- a/tools/patman/patchwork.py
+++ b/tools/patman/patchwork.py
@@ -449,6 +449,52 @@  class Patchwork:
         """
         return await self._request(client, f'series/{link}/')
 
+    async def get_series_mbox(self, client, link):
+        """Download the raw mbox file for a series.
+
+        The mbox URL lives directly under '/series/<link>/mbox/' rather
+        than the JSON API, so this fetches the bytes directly rather
+        than routing through self._request().
+
+        Args:
+            client (aiohttp.ClientSession): Session to use
+            link (str): Patchwork series link/ID
+
+        Returns:
+            bytes: Raw mbox content
+
+        Raises:
+            ValueError: if the download fails
+        """
+        self.request_count += 1
+        full_url = f'{self.url}/series/{link}/mbox/'
+        async with self.semaphore:
+            async with client.get(full_url) as response:
+                if response.status != 200:
+                    raise ValueError(
+                        f'Failed to download mbox: HTTP {response.status}')
+                return await response.read()
+
+    async def search_patches(self, client, query, project_id=None,
+                             per_page=20):
+        """Search patches by free-text query, most-recent first.
+
+        Args:
+            client (aiohttp.ClientSession): Session to use
+            query (str): Text to match against patch titles
+            project_id (int): Project ID to scope to, or None to use
+                self.proj_id
+            per_page (int): Maximum number of results to return
+
+        Returns:
+            list of dict: Patch records matching the query
+        """
+        if project_id is None:
+            project_id = self.proj_id
+        subpath = (f'patches/?project={project_id}&q={query}'
+                   f'&order=-date&per_page={per_page}')
+        return await self._request(client, subpath)
+
     async def get_patch(self, client, patch_id):
         """Read information about a patch
 
diff --git a/tools/patman/review.py b/tools/patman/review.py
index 09614c85ed7..eb2e46a0431 100644
--- a/tools/patman/review.py
+++ b/tools/patman/review.py
@@ -100,11 +100,11 @@  class ReviewContext:  # pylint: disable=R0902
         return self.series_data.get('date', '')
 
 
-async def fetch_mbox(pwork_url, link):
+async def fetch_mbox(pwork, link):
     """Download the series mbox file from patchwork
 
     Args:
-        pwork_url (str): Patchwork server URL
+        pwork (Patchwork): Patchwork instance to fetch from
         link (str): Patchwork series link/ID
 
     Returns:
@@ -113,19 +113,13 @@  async def fetch_mbox(pwork_url, link):
     Raises:
         ValueError: if the download fails
     """
-    url = f'{pwork_url}/series/{link}/mbox/'
-    tout.notice(f'Downloading mbox from {url}')
+    tout.notice(f'Downloading mbox for series {link} from {pwork.url}')
     mbox_path = os.path.join(tempfile.gettempdir(),
                              f'patman_review_{link}.mbox')
     async with aiohttp.ClientSession() as client:
-        async with client.get(url) as response:
-            if response.status != 200:
-                raise ValueError(
-                    f'Failed to download mbox: HTTP {response.status}')
-
-            content = await response.read()
-            if not content:
-                raise ValueError(f'Empty mbox downloaded from {url}')
+        content = await pwork.get_series_mbox(client, link)
+    if not content:
+        raise ValueError(f'Empty mbox downloaded for series {link}')
 
     tools.write_file(mbox_path, content)
     tout.notice(f'Downloaded {len(content)} bytes to {mbox_path}')
@@ -205,7 +199,7 @@  IMPORTANT:
 '''
 
 
-async def apply_series(pwork_url, link, branch_name, upstream_branch,
+async def apply_series(pwork, link, branch_name, upstream_branch,
                        repo_path):
     """Download and apply a patch series to a new local branch
 
@@ -213,7 +207,7 @@  async def apply_series(pwork_url, link, branch_name, upstream_branch,
     resolution.
 
     Args:
-        pwork_url (str): Patchwork server URL
+        pwork (Patchwork): Patchwork instance to fetch from
         link (str): Patchwork series link/ID
         branch_name (str): Name for the new branch
         upstream_branch (str): Branch to base from
@@ -228,7 +222,7 @@  async def apply_series(pwork_url, link, branch_name, upstream_branch,
         return False, None
 
     # Download the mbox
-    mbox_path = await fetch_mbox(pwork_url, link)
+    mbox_path = await fetch_mbox(pwork, link)
 
     # Build the prompt and run the agent
     prompt = _build_apply_prompt(mbox_path, branch_name, upstream_branch)
@@ -1058,11 +1052,11 @@  def review_patches_sync(ctx):
     return loop.run_until_complete(review_patches(ctx))
 
 
-def apply_series_sync(pwork_url, link, branch_name, upstream_branch, repo_path):
+def apply_series_sync(pwork, link, branch_name, upstream_branch, repo_path):
     """Synchronous wrapper for apply_series()
 
     Args:
-        pwork_url (str): Patchwork server URL
+        pwork (Patchwork): Patchwork instance to fetch from
         link (str): Patchwork series link/ID
         branch_name (str): Name for the new branch
         upstream_branch (str): Branch to base from
@@ -1073,7 +1067,7 @@  def apply_series_sync(pwork_url, link, branch_name, upstream_branch, repo_path):
     """
     loop = asyncio.get_event_loop()
     return loop.run_until_complete(apply_series(
-        pwork_url, link, branch_name, upstream_branch, repo_path))
+        pwork, link, branch_name, upstream_branch, repo_path))
 
 
 def search_series(pwork, title):
@@ -1273,12 +1267,10 @@  def _do_learn_voice(args, pwork):
 
     if pwork and pwork.proj_id:
         async def _get_list_email():
-            async with aiohttp.ClientSession() as client:
-                # pylint: disable=W0212
-                projects = await pwork._request(client, 'projects/')
-                for proj in projects:
-                    if proj['id'] == pwork.proj_id:
-                        return proj.get('list_email')
+            projects = await pwork.get_projects()
+            for proj in projects:
+                if proj['id'] == pwork.proj_id:
+                    return proj.get('list_email')
             return None
 
         loop = asyncio.get_event_loop()
@@ -1600,7 +1592,7 @@  def _apply_and_check(ctx, link):
     """
     branch_name = f'review{ctx.series_id}'
     repo_path = gitutil.get_top_level()
-    success, branch_name = apply_series_sync(ctx.pwork.url, link, branch_name,
+    success, branch_name = apply_series_sync(ctx.pwork, link, branch_name,
         ctx.upstream_branch, repo_path)
 
     if success: