[PATCH b4 0/3] review: avoid downloading logs and fix URLs

[PATCH b4 0/3] review: avoid downloading logs and fix URLs

[Matthieu Baerts (NGI0)]

The build-in Sashiko and Patchwork support is nice!

Here are just some small fixes:

- Patch 1: avoid downloading the review logs that are not used.

- Patch 2: fix Sashiko URLs that were leading to a 404.

- Patch 3: use Patchwork target URL instead of the API one.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Matthieu Baerts (NGI0) (3):
      review: sashiko: switch to patchset endpoint
      review: sashiko: fix URLs
      review: patchwork: use the target URL

 src/b4/review/checks.py         | 9 ++++++---
 src/tests/test_review_checks.py | 4 ++--
 2 files changed, 8 insertions(+), 5 deletions(-)
---
base-commit: d5d981426ead3f490713ef5d2fd1aa3d0f13b005
change-id: 20260428-sashiko-patchset-api-277ecc2ffbd4

Best regards,
--  
Matthieu Baerts (NGI0) <matttbe@kernel.org>

[PATCH b4 1/3] review: sashiko: switch to patchset endpoint

[Matthieu Baerts (NGI0)]

The 'patch' endpoint also contains the logs [1], which can be huge -- up
to a few MBytes -- and not used here.

That's why a new 'patchset' endpoint has been introduced with the same
content, but without the logs that can be retrieved from 'review_log' if
needed [2].

Link: https://github.com/sashiko-dev/sashiko/issues/57 [1]
Link: https://github.com/sashiko-dev/sashiko/commit/3dbb448ed0a4 [2]
Assisted-by: My-slow-DSL-connection
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 src/b4/review/checks.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/b4/review/checks.py b/src/b4/review/checks.py
index 36c0780..2ff825f 100644
--- a/src/b4/review/checks.py
+++ b/src/b4/review/checks.py
@@ -361,7 +361,7 @@ def _fetch_sashiko_patchset(msgid: str, sashiko_url: str) -> Optional[Dict[str,
     if msgid in _sashiko_patchset_cache:
         return _sashiko_patchset_cache[msgid]
 
-    url = f'{sashiko_url.rstrip("/")}/api/patch'
+    url = f'{sashiko_url.rstrip("/")}/api/patchset'
     try:
         session = b4.get_requests_session()
         resp = session.get(url, params={'id': msgid}, timeout=30)

-- 
2.53.0

[PATCH b4 2/3] review: sashiko: fix URLs

[Matthieu Baerts (NGI0)]

The displayed URL is currently leading to a 404, e.g.

  https://sashiko.dev/patch/93

I don't know if this URL was working at some points, but it is no longer
now. This one works:

  https://sashiko.dev/#/patchset/93

And in case of a series, ?part=X can be used, e.g.

  https://sashiko.dev/#/patchset/93?part=2

Assisted-by: My-reading-glasses
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 src/b4/review/checks.py         | 5 ++++-
 src/tests/test_review_checks.py | 4 ++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/b4/review/checks.py b/src/b4/review/checks.py
index 2ff825f..942935e 100644
--- a/src/b4/review/checks.py
+++ b/src/b4/review/checks.py
@@ -462,7 +462,7 @@ def _run_builtin_sashiko(msg: EmailMessage, sashiko_url: str) -> List[Dict[str,
     reviews = data.get('reviews', [])
     patches = data.get('patches', [])
     base_url = sashiko_url.rstrip('/')
-    patchset_url = f'{base_url}/patch/{ps_id}' if ps_id else ''
+    patchset_url = f'{base_url}/#/patchset/{ps_id if ps_id else msgid}'
 
     # Build a map from patch message-id to sashiko patch id
     patch_id_by_msgid: Dict[str, int] = {}
@@ -526,8 +526,10 @@ def _run_builtin_sashiko(msg: EmailMessage, sashiko_url: str) -> List[Dict[str,
     if sashiko_patch_id is None:
         return []
 
+    i = 1
     for review in reviews:
         if review.get('patch_id') == sashiko_patch_id:
+            patchset_url += f'?part={i}'
             review_status = review.get('status', '')
             if review_status == 'Skipped':
                 result_msg = review.get('result', '') or 'Skipped'
@@ -570,6 +572,7 @@ def _run_builtin_sashiko(msg: EmailMessage, sashiko_url: str) -> List[Dict[str,
             if findings:
                 result['details'] = json.dumps(findings)
             return [result]
+        i += 1
 
     # No review found for this patch
     return [
diff --git a/src/tests/test_review_checks.py b/src/tests/test_review_checks.py
index 1469bf3..ad0f222 100644
--- a/src/tests/test_review_checks.py
+++ b/src/tests/test_review_checks.py
@@ -861,7 +861,7 @@ class TestRunBuiltinSashiko:
         assert '1 critical' in results[0]['summary']
         assert '1 high' in results[0]['summary']
         assert '1 low' in results[0]['summary']
-        assert results[0]['url'] == 'https://sashiko.dev/patch/93'
+        assert results[0]['url'] == 'https://sashiko.dev/#/patchset/93'
         # Details should be valid JSON
         details = json.loads(results[0]['details'])
         assert len(details) == 3  # 1 low + 1 critical + 1 high
@@ -992,7 +992,7 @@ class TestRunBuiltinSashiko:
         msg = _make_msg(msgid='patch1@example.com')
         results = checks._run_builtin_sashiko(msg, 'https://sashiko.dev/')
         # Trailing slash should not cause double slash
-        assert results[0]['url'] == 'https://sashiko.dev/patch/93'
+        assert results[0]['url'] == 'https://sashiko.dev/#/patchset/93?part=1'
 
 
 class TestSashikoAutoWire:

-- 
2.53.0

[PATCH b4 3/3] review: patchwork: use the target URL

[Matthieu Baerts (NGI0)]

The 'url' field from the 'checks' endpoint is pointing to the Patchwork
API, not very interesting to display, e.g.

  https://patchwork.kernel.org/api/patches/14521024/checks/9433123/

Instead, use the 'target_url' which typically points to something useful
linked to the results: more details, logs, etc. e.g.

  https://github.com/multipath-tcp/mptcp_net-next/actions/runs/24274365064

Assisted-by: Not-my-dog
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 src/b4/review/checks.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/b4/review/checks.py b/src/b4/review/checks.py
index 942935e..96c769e 100644
--- a/src/b4/review/checks.py
+++ b/src/b4/review/checks.py
@@ -333,7 +333,7 @@ def _run_builtin_patchwork(
                 'status': status,
                 'state': state,
                 'description': check.get('description', ''),
-                'url': check.get('url', ''),
+                'url': check.get('target_url', ''),
             }
         )
 

-- 
2.53.0