feat: emit a warning when using a list() method returns max

JohnVillalovos · JohnVillalovos · commit 0cbddceb8320 · 2022-02-03T14:33:54.000-08:00
A common cause of issues filed and questions raised is that a user
will call a `list()` method and only get 20 items. As this is the
default maximum of items that will be returned from a `list()` method.

To help with this we now emit a warning when the result from a
`list()` method is greater-than or equal to 20 (or the specified
`per_page` value) and the user is not using either `all=True`,
`as_list=False`, or `page=X`.
diff --git a/gitlab/client.py b/gitlab/client.py
@@ -18,12 +18,14 @@
 
 import os
 import time
+import warnings
 from typing import Any, cast, Dict, List, Optional, Tuple, TYPE_CHECKING, Union
 
 import requests
 import requests.utils
 from requests_toolbelt.multipart.encoder import MultipartEncoder  # type: ignore
 
+import gitlab
 import gitlab.config
 import gitlab.const
 import gitlab.exceptions
@@ -35,6 +37,14 @@
     "{source!r} to {target!r}"
 )
 
+# https://docs.gitlab.com/ee/api/#offset-based-pagination
+DEFAULT_PER_PAGE = 20
+MAXIMUM_PER_PAGE = 100
+_PAGINATION_URL = (
+    f"https://python-gitlab.readthedocs.io/en/v{gitlab.__version__}/"
+    f"api-usage.html#pagination"
+)
+
 
 class Gitlab:
     """Represents a GitLab server connection.
@@ -816,9 +826,22 @@ def http_list(
         if get_all is True and as_list is True:
             return list(GitlabList(self, url, query_data, **kwargs))
 
+        per_page = min(kwargs.get("per_page", DEFAULT_PER_PAGE), MAXIMUM_PER_PAGE)
         if page or as_list is True:
             # pagination requested, we return a list
-            return list(GitlabList(self, url, query_data, get_next=False, **kwargs))
+            items = list(GitlabList(self, url, query_data, get_next=False, **kwargs))
+            if page is None and len(items) >= per_page:
+                # Warn the user that they are only going to retrieve `per_page` maximum
+                # items.  This is a common cause of issues filed.
+                warnings.warn(
+                    (
+                        f"Calling a `list()` method without specifying `all=True` or "
+                        f"`as_list=False` will return a maximum of {per_page} items. "
+                        f"See {_PAGINATION_URL} for more details"
+                    ),
+                    UserWarning,
+                )
+            return items
 
         # No pagination, generator requested
         return GitlabList(self, url, query_data, **kwargs)
diff --git a/tests/unit/test_gitlab_http_methods.py b/tests/unit/test_gitlab_http_methods.py
@@ -1,3 +1,5 @@
+import warnings
+
 import pytest
 import requests
 import responses
@@ -329,20 +331,83 @@ def test_list_request(gl):
         match=MATCH_EMPTY_QUERY_PARAMS,
     )
 
-    result = gl.http_list("/projects", as_list=True)
+    with warnings.catch_warnings(record=True) as caught_warnings:
+        result = gl.http_list("/projects", as_list=True)
+    assert len(caught_warnings) == 0
     assert isinstance(result, list)
     assert len(result) == 1
 
     result = gl.http_list("/projects", as_list=False)
     assert isinstance(result, GitlabList)
-    assert len(result) == 1
+    assert len(list(result)) == 1
 
     result = gl.http_list("/projects", all=True)
     assert isinstance(result, list)
     assert len(result) == 1
     assert responses.assert_call_count(url, 3) is True
 
 
+@responses.activate
+def test_list_request_pagination_warning(gl):
+    url = "http://localhost/api/v4/projects"
+    responses.add(
+        method=responses.GET,
+        url=url,
+        json=[
+            {"name": "project01"},
+            {"name": "project02"},
+            {"name": "project03"},
+            {"name": "project04"},
+            {"name": "project05"},
+            {"name": "project06"},
+            {"name": "project07"},
+            {"name": "project08"},
+            {"name": "project09"},
+            {"name": "project10"},
+            {"name": "project11"},
+            {"name": "project12"},
+            {"name": "project13"},
+            {"name": "project14"},
+            {"name": "project15"},
+            {"name": "project16"},
+            {"name": "project17"},
+            {"name": "project18"},
+            {"name": "project19"},
+            {"name": "project20"},
+        ],
+        headers={"X-Total": "1"},
+        status=200,
+        match=MATCH_EMPTY_QUERY_PARAMS,
+    )
+
+    with warnings.catch_warnings(record=True) as caught_warnings:
+        result = gl.http_list("/projects", as_list=True)
+    assert len(caught_warnings) == 1
+    warning = caught_warnings[0]
+    assert isinstance(warning.message, UserWarning)
+    message = str(caught_warnings[0].message)
+    assert "Calling" in message
+    assert "return a maximum of" in message
+    assert "readthedocs" in message
+
+    assert isinstance(result, list)
+    assert len(result) == 20
+
+    with warnings.catch_warnings(record=True) as caught_warnings:
+        result = gl.http_list("/projects", as_list=False)
+    assert len(caught_warnings) == 0
+    assert isinstance(result, GitlabList)
+    assert len(list(result)) == 20
+
+    with warnings.catch_warnings(record=True) as caught_warnings:
+        result = gl.http_list("/projects", all=True)
+    assert len(caught_warnings) == 0
+
+    assert isinstance(result, list)
+    assert len(result) == 20
+    assert responses.assert_call_count(url, 3) is True
+
+
 @responses.activate
 def test_list_request_404(gl):
     url = "http://localhost/api/v4/not_there"
