(magpie) branch main updated: fix(tools/gitlab): address review follow-up on pagination docstring, short pages, and issue template (#1441)

[po...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/magpie.git


The following commit(s) were added to refs/heads/main by this push:
     new 3d0f4920 fix(tools/gitlab): address review follow-up on pagination docstring, short pages, and issue template (#1441)
3d0f4920 is described below

commit 3d0f49207d3ffe01d555a012365151571477bc44
Author: Vardhman Gupta <11...@users.noreply.github.com>
AuthorDate: Mon Sep 28 12:26:26 2026 +0530

    fix(tools/gitlab): address review follow-up on pagination docstring, short pages, and issue template (#1441)
    
    * fix(tools/gitlab): address review follow-up on pagination docstring, short pages, and issue template
    
    * docs(tools/gitlab): restore the doctoc-generated TOC title in issue-template.md
    
    Generated-by: Claude Opus 5
    
    ---------
    
    Co-authored-by: Jarek Potiuk <po...@apache.org>
---
 tools/gitlab/issue-template.md           |  9 ++++-----
 tools/gitlab/src/magpie_gitlab/client.py | 12 +++++-------
 tools/gitlab/tests/test_client.py        | 16 +++++++++++++++-
 3 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/tools/gitlab/issue-template.md b/tools/gitlab/issue-template.md
index 51e83c7a..c2f260d1 100644
--- a/tools/gitlab/issue-template.md
+++ b/tools/gitlab/issue-template.md
@@ -1,3 +1,6 @@
+<!-- SPDX-License-Identifier: Apache-2.0
+     https://www.apache.org/licenses/LICENSE-2.0 -->
+
 <!-- START doctoc generated TOC please keep comment here to allow auto update -->
 <!-- DON'T EDIT THIS SECTION, INSTEAD RE-RUN doctoc TO UPDATE -->
 **Table of Contents**  *generated with [DocToc](https://github.com/thlorenz/doctoc)*
@@ -7,9 +10,6 @@
 
 <!-- END doctoc generated TOC please keep comment here to allow auto update -->
 
-<!-- SPDX-License-Identifier: Apache-2.0
-     https://www.apache.org/licenses/LICENSE-2.0 -->
-
 # GitLab Issue Template
 
 Schema for GitLab issues and merge request bodies.
@@ -21,5 +21,4 @@ GitLab Flavored Markdown (GLFM) is used across all issue and MR descriptions:
 - **References**: Issues are referenced with `#ID` and MRs with `!ID`.
 - **Labels**: Scoped labels often use `::` (e.g., `workflow::in-review`).
 
-Ensure that the adapter correctly parses these specific constructs
-when reading issue bodies.
+Descriptive reference for Markdown constructs commonly present in GitLab issue and merge request payloads.
diff --git a/tools/gitlab/src/magpie_gitlab/client.py b/tools/gitlab/src/magpie_gitlab/client.py
index e02d4e70..3ac72de1 100644
--- a/tools/gitlab/src/magpie_gitlab/client.py
+++ b/tools/gitlab/src/magpie_gitlab/client.py
@@ -18,7 +18,6 @@
 from __future__ import annotations
 
 import json
-import math
 import os
 import sys
 import urllib.error
@@ -208,10 +207,6 @@ def get_paged_json(
     current_url: str | None = f"{url}{separator}per_page=100" if "per_page=" not in url else url
     pages_fetched = 0
 
-    target_pages: int | None = max_pages
-    if limit is not None:
-        target_pages = max(1, math.ceil(limit / 100))
-
     opener = _build_opener()
     while current_url:
         request = urllib.request.Request(current_url, headers=headers, method="GET")
@@ -233,14 +228,17 @@ def get_paged_json(
                     items = items[:limit]
                     break
 
-                if target_pages is not None and pages_fetched >= target_pages:
-                    if has_more and limit is None:
+                if limit is None and max_pages is not None and pages_fetched >= max_pages:
+                    if has_more:
                         print(
                             f"[magpie-gitlab] Note: Results capped at {len(items)} items ({pages_fetched} pages); use --limit to fetch more.",
                             file=sys.stderr,
                         )
                     break
 
+                if not data:
+                    break
+
                 if has_more and isinstance(next_page, str):
                     parsed = urllib.parse.urlparse(current_url)
                     query = urllib.parse.parse_qs(parsed.query)
diff --git a/tools/gitlab/tests/test_client.py b/tools/gitlab/tests/test_client.py
index 664b0a78..9711e4ee 100644
--- a/tools/gitlab/tests/test_client.py
+++ b/tools/gitlab/tests/test_client.py
@@ -349,7 +349,7 @@ def test_get_paged_json_max_pages(mock_urlopen, mock_env, capsys):
 
 
 def test_get_paged_json_with_limit_fewer_than_page(mock_urlopen, mock_env, capsys):
-    """Limit fewer than page size fetches only 1 page, caps items, and emits notice if more exist."""
+    """Limit fewer than page size fetches only 1 page, caps items, and emits no notice."""
     data = [{"id": i} for i in range(100)]
     mock_urlopen.return_value = build_mock_response(data, headers={"X-Next-Page": "2"})
 
@@ -361,6 +361,20 @@ def test_get_paged_json_with_limit_fewer_than_page(mock_urlopen, mock_env, capsy
     assert err == ""
 
 
+def test_get_paged_json_short_page_continues_until_limit(mock_urlopen, mock_env):
+    """When pages return fewer items than default per_page, pagination continues until limit is met."""
+    page1 = build_mock_response([{"id": 1}, {"id": 2}], headers={"X-Next-Page": "2"})
+    page2 = build_mock_response([{"id": 3}, {"id": 4}], headers={"X-Next-Page": "3"})
+    page3 = build_mock_response([{"id": 5}, {"id": 6}], headers={"X-Next-Page": ""})
+    mock_urlopen.side_effect = [page1, page2, page3]
+
+    cfg = load_config()
+    items = get_paged_json("https://gitlab.example.com/api/v4/projects/test/issues", cfg, limit=5)
+    assert len(items) == 5
+    assert [x["id"] for x in items] == [1, 2, 3, 4, 5]
+    assert mock_urlopen.call_count == 3
+
+
 def test_get_paged_json_with_limit_multi_page(mock_urlopen, mock_env):
     """Limit spanning multiple pages only fetches the required number of pages."""
     page1 = build_mock_response([{"id": i} for i in range(100)], headers={"X-Next-Page": "2"})