Skip to content

Commit cdc6605

Browse files
JohnVillalovosJohnVillalovos
committed
feat(client): introduce iterator=True and deprecate as_list=False in list()
`as_list=False` is confusing as it doesn't explain what is being returned. Replace it with `iterator=True` which more clearly explains to the user that an iterator/generator will be returned. This maintains backward compatibility with `as_list` but does issue a DeprecationWarning if `as_list` is set.
1 parent 5ae18d0 commit cdc6605

16 files changed

+99
-63
lines changed

‎docs/api-usage.rst

+13-9
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,13 @@ Examples:
9393
.. code-block:: python
9494
9595
# list all the projects
96-
projects = gl.projects.list(as_list=False)
96+
projects = gl.projects.list(iterator=True)
9797
for project in projects:
9898
print(project)
9999
100100
# get the group with id == 2
101101
group = gl.groups.get(2)
102-
for project in group.projects.list(as_list=False):
102+
for project in group.projects.list(iterator=True):
103103
print(project)
104104
105105
# create a new user
@@ -109,7 +109,7 @@ Examples:
109109
110110
.. warning::
111111
Calling ``list()`` without any arguments will by default not return the complete list
112-
of items. Use either the ``all=True`` or ``as_list=False`` parameters to get all the
112+
of items. Use either the ``all=True`` or ``iterator=True`` parameters to get all the
113113
items when using listing methods. See the :ref:`pagination` section for more
114114
information.
115115

@@ -156,9 +156,9 @@ conflict with python or python-gitlab when using them as kwargs:
156156

157157
.. code-block:: python
158158
159-
gl.user_activities.list(from='2019-01-01', as_list=False) ## invalid
159+
gl.user_activities.list(from='2019-01-01', iterator=True) ## invalid
160160
161-
gl.user_activities.list(query_parameters={'from': '2019-01-01'}, as_list=False) # OK
161+
gl.user_activities.list(query_parameters={'from': '2019-01-01'}, iterator=True) # OK
162162
163163
Gitlab Objects
164164
==============
@@ -282,13 +282,13 @@ order options. At the time of writing, only ``order_by="id"`` works.
282282
Reference:
283283
https://docs.gitlab.com/ce/api/README.html#keyset-based-pagination
284284

285-
``list()`` methods can also return a generator object which will handle the
286-
next calls to the API when required. This is the recommended way to iterate
287-
through a large number of items:
285+
``list()`` methods can also return a generator object, by passing the argument
286+
``iterator=True``, which will handle the next calls to the API when required. This
287+
is the recommended way to iterate through a large number of items:
288288

289289
.. code-block:: python
290290
291-
items = gl.groups.list(as_list=False)
291+
items = gl.groups.list(iterator=True)
292292
for item in items:
293293
print(item.attributes)
294294
@@ -310,6 +310,10 @@ The generator exposes extra listing information as received from the server:
310310
For more information see:
311311
https://docs.gitlab.com/ee/user/gitlab_com/index.html#pagination-response-headers
312312

313+
.. note::
314+
Prior to python-gitlab 3.6.0 the argument ``as_list`` was used instead of
315+
``iterator``. ``as_list=False`` is the equivalent of ``iterator=True``.
316+
313317
Sudo
314318
====
315319

‎docs/gl_objects/search.rst

+2-2
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,13 @@ The ``search()`` methods implement the pagination support::
6363

6464
# get a generator that will automatically make required API calls for
6565
# pagination
66-
for item in gl.search(gitlab.const.SEARCH_SCOPE_ISSUES, search_str, as_list=False):
66+
for item in gl.search(gitlab.const.SEARCH_SCOPE_ISSUES, search_str, iterator=True):
6767
do_something(item)
6868

6969
The search API doesn't return objects, but dicts. If you need to act on
7070
objects, you need to create them explicitly::
7171

72-
for item in gl.search(gitlab.const.SEARCH_SCOPE_ISSUES, search_str, as_list=False):
72+
for item in gl.search(gitlab.const.SEARCH_SCOPE_ISSUES, search_str, iterator=True):
7373
issue_project = gl.projects.get(item['project_id'], lazy=True)
7474
issue = issue_project.issues.get(item['iid'])
7575
issue.state = 'closed'

‎docs/gl_objects/users.rst

+1-1
Original file line numberDiff line numberDiff line change
@@ -413,4 +413,4 @@ Get the users activities::
413413

414414
activities = gl.user_activities.list(
415415
query_parameters={'from': '2018-07-01'},
416-
all=True, as_list=False)
416+
all=True, iterator=True)

‎gitlab/client.py

+24-7
Original file line numberDiff line numberDiff line change
@@ -807,7 +807,9 @@ def http_list(
807807
self,
808808
path: str,
809809
query_data: Optional[Dict[str, Any]] = None,
810-
as_list: Optional[bool] = None,
810+
*,
811+
as_list: Optional[bool] = None, # Deprecated in favor of `iterator`
812+
iterator: Optional[bool] = None,
811813
**kwargs: Any,
812814
) -> Union["GitlabList", List[Dict[str, Any]]]:
813815
"""Make a GET request to the Gitlab server for list-oriented queries.
@@ -816,12 +818,13 @@ def http_list(
816818
path: Path or full URL to query ('/projects' or
817819
'http://whatever/v4/api/projects')
818820
query_data: Data to send as query parameters
821+
iterator: Indicate if should return a generator (True)
819822
**kwargs: Extra options to send to the server (e.g. sudo, page,
820823
per_page)
821824
822825
Returns:
823-
A list of the objects returned by the server. If `as_list` is
824-
False and no pagination-related arguments (`page`, `per_page`,
826+
A list of the objects returned by the server. If `iterator` is
827+
True and no pagination-related arguments (`page`, `per_page`,
825828
`all`) are defined then a GitlabList object (generator) is returned
826829
instead. This object will make API calls when needed to fetch the
827830
next items from the server.
@@ -832,15 +835,29 @@ def http_list(
832835
"""
833836
query_data = query_data or {}
834837

835-
# In case we want to change the default behavior at some point
836-
as_list = True if as_list is None else as_list
838+
# Don't allow both `as_list` and `iterator` to be set.
839+
if as_list is not None and iterator is not None:
840+
raise ValueError(
841+
"Only one of `as_list` or `iterator` can be used. "
842+
"Use `iterator` instead of `as_list`. `as_list` is deprecated."
843+
)
844+
845+
if as_list is not None:
846+
iterator = not as_list
847+
utils.warn(
848+
message=(
849+
f"`as_list={as_list}` is deprecated and will be removed in a "
850+
f"future version. Use `iterator={iterator}` instead."
851+
),
852+
category=DeprecationWarning,
853+
)
837854

838855
get_all = kwargs.pop("all", None)
839856
url = self._build_url(path)
840857

841858
page = kwargs.get("page")
842859

843-
if as_list is False:
860+
if iterator:
844861
# Generator requested
845862
return GitlabList(self, url, query_data, **kwargs)
846863

@@ -879,7 +896,7 @@ def should_emit_warning() -> bool:
879896
utils.warn(
880897
message=(
881898
f"Calling a `list()` method without specifying `all=True` or "
882-
f"`as_list=False` will return a maximum of {gl_list.per_page} items. "
899+
f"`iterator=True` will return a maximum of {gl_list.per_page} items. "
883900
f"Your query returned {len(items)} of {total_items} items. See "
884901
f"{_PAGINATION_URL} for more details. If this was done intentionally, "
885902
f"then this warning can be supressed by adding the argument "

‎gitlab/mixins.py

+2-4
Original file line numberDiff line numberDiff line change
@@ -201,12 +201,12 @@ def list(self, **kwargs: Any) -> Union[base.RESTObjectList, List[base.RESTObject
201201
all: If True, return all the items, without pagination
202202
per_page: Number of items to retrieve per request
203203
page: ID of the page to return (starts with page 1)
204-
as_list: If set to False and no pagination option is
204+
iterator: If set to True and no pagination option is
205205
defined, return a generator instead of a list
206206
**kwargs: Extra options to send to the server (e.g. sudo)
207207
208208
Returns:
209-
The list of objects, or a generator if `as_list` is False
209+
The list of objects, or a generator if `iterator` is True
210210
211211
Raises:
212212
GitlabAuthenticationError: If authentication is not correct
@@ -846,8 +846,6 @@ def participants(self, **kwargs: Any) -> Dict[str, Any]:
846846
all: If True, return all the items, without pagination
847847
per_page: Number of items to retrieve per request
848848
page: ID of the page to return (starts with page 1)
849-
as_list: If set to False and no pagination option is
850-
defined, return a generator instead of a list
851849
**kwargs: Extra options to send to the server (e.g. sudo)
852850
853851
Raises:

‎gitlab/v4/objects/ldap.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,12 @@ def list(self, **kwargs: Any) -> Union[List[LDAPGroup], RESTObjectList]:
2626
all: If True, return all the items, without pagination
2727
per_page: Number of items to retrieve per request
2828
page: ID of the page to return (starts with page 1)
29-
as_list: If set to False and no pagination option is
29+
iterator: If set to True and no pagination option is
3030
defined, return a generator instead of a list
3131
**kwargs: Extra options to send to the server (e.g. sudo)
3232
3333
Returns:
34-
The list of objects, or a generator if `as_list` is False
34+
The list of objects, or a generator if `iterator` is True
3535
3636
Raises:
3737
GitlabAuthenticationError: If authentication is not correct

‎gitlab/v4/objects/merge_requests.py

+2-6
Original file line numberDiff line numberDiff line change
@@ -199,8 +199,6 @@ def closes_issues(self, **kwargs: Any) -> RESTObjectList:
199199
all: If True, return all the items, without pagination
200200
per_page: Number of items to retrieve per request
201201
page: ID of the page to return (starts with page 1)
202-
as_list: If set to False and no pagination option is
203-
defined, return a generator instead of a list
204202
**kwargs: Extra options to send to the server (e.g. sudo)
205203
206204
Raises:
@@ -211,7 +209,7 @@ def closes_issues(self, **kwargs: Any) -> RESTObjectList:
211209
List of issues
212210
"""
213211
path = f"{self.manager.path}/{self.encoded_id}/closes_issues"
214-
data_list = self.manager.gitlab.http_list(path, as_list=False, **kwargs)
212+
data_list = self.manager.gitlab.http_list(path, iterator=True, **kwargs)
215213
if TYPE_CHECKING:
216214
assert isinstance(data_list, gitlab.GitlabList)
217215
manager = ProjectIssueManager(self.manager.gitlab, parent=self.manager._parent)
@@ -226,8 +224,6 @@ def commits(self, **kwargs: Any) -> RESTObjectList:
226224
all: If True, return all the items, without pagination
227225
per_page: Number of items to retrieve per request
228226
page: ID of the page to return (starts with page 1)
229-
as_list: If set to False and no pagination option is
230-
defined, return a generator instead of a list
231227
**kwargs: Extra options to send to the server (e.g. sudo)
232228
233229
Raises:
@@ -239,7 +235,7 @@ def commits(self, **kwargs: Any) -> RESTObjectList:
239235
"""
240236

241237
path = f"{self.manager.path}/{self.encoded_id}/commits"
242-
data_list = self.manager.gitlab.http_list(path, as_list=False, **kwargs)
238+
data_list = self.manager.gitlab.http_list(path, iterator=True, **kwargs)
243239
if TYPE_CHECKING:
244240
assert isinstance(data_list, gitlab.GitlabList)
245241
manager = ProjectCommitManager(self.manager.gitlab, parent=self.manager._parent)

‎gitlab/v4/objects/milestones.py

+4-12
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,6 @@ def issues(self, **kwargs: Any) -> RESTObjectList:
3333
all: If True, return all the items, without pagination
3434
per_page: Number of items to retrieve per request
3535
page: ID of the page to return (starts with page 1)
36-
as_list: If set to False and no pagination option is
37-
defined, return a generator instead of a list
3836
**kwargs: Extra options to send to the server (e.g. sudo)
3937
4038
Raises:
@@ -46,7 +44,7 @@ def issues(self, **kwargs: Any) -> RESTObjectList:
4644
"""
4745

4846
path = f"{self.manager.path}/{self.encoded_id}/issues"
49-
data_list = self.manager.gitlab.http_list(path, as_list=False, **kwargs)
47+
data_list = self.manager.gitlab.http_list(path, iterator=True, **kwargs)
5048
if TYPE_CHECKING:
5149
assert isinstance(data_list, RESTObjectList)
5250
manager = GroupIssueManager(self.manager.gitlab, parent=self.manager._parent)
@@ -62,8 +60,6 @@ def merge_requests(self, **kwargs: Any) -> RESTObjectList:
6260
all: If True, return all the items, without pagination
6361
per_page: Number of items to retrieve per request
6462
page: ID of the page to return (starts with page 1)
65-
as_list: If set to False and no pagination option is
66-
defined, return a generator instead of a list
6763
**kwargs: Extra options to send to the server (e.g. sudo)
6864
6965
Raises:
@@ -74,7 +70,7 @@ def merge_requests(self, **kwargs: Any) -> RESTObjectList:
7470
The list of merge requests
7571
"""
7672
path = f"{self.manager.path}/{self.encoded_id}/merge_requests"
77-
data_list = self.manager.gitlab.http_list(path, as_list=False, **kwargs)
73+
data_list = self.manager.gitlab.http_list(path, iterator=True, **kwargs)
7874
if TYPE_CHECKING:
7975
assert isinstance(data_list, RESTObjectList)
8076
manager = GroupIssueManager(self.manager.gitlab, parent=self.manager._parent)
@@ -114,8 +110,6 @@ def issues(self, **kwargs: Any) -> RESTObjectList:
114110
all: If True, return all the items, without pagination
115111
per_page: Number of items to retrieve per request
116112
page: ID of the page to return (starts with page 1)
117-
as_list: If set to False and no pagination option is
118-
defined, return a generator instead of a list
119113
**kwargs: Extra options to send to the server (e.g. sudo)
120114
121115
Raises:
@@ -127,7 +121,7 @@ def issues(self, **kwargs: Any) -> RESTObjectList:
127121
"""
128122

129123
path = f"{self.manager.path}/{self.encoded_id}/issues"
130-
data_list = self.manager.gitlab.http_list(path, as_list=False, **kwargs)
124+
data_list = self.manager.gitlab.http_list(path, iterator=True, **kwargs)
131125
if TYPE_CHECKING:
132126
assert isinstance(data_list, RESTObjectList)
133127
manager = ProjectIssueManager(self.manager.gitlab, parent=self.manager._parent)
@@ -143,8 +137,6 @@ def merge_requests(self, **kwargs: Any) -> RESTObjectList:
143137
all: If True, return all the items, without pagination
144138
per_page: Number of items to retrieve per request
145139
page: ID of the page to return (starts with page 1)
146-
as_list: If set to False and no pagination option is
147-
defined, return a generator instead of a list
148140
**kwargs: Extra options to send to the server (e.g. sudo)
149141
150142
Raises:
@@ -155,7 +147,7 @@ def merge_requests(self, **kwargs: Any) -> RESTObjectList:
155147
The list of merge requests
156148
"""
157149
path = f"{self.manager.path}/{self.encoded_id}/merge_requests"
158-
data_list = self.manager.gitlab.http_list(path, as_list=False, **kwargs)
150+
data_list = self.manager.gitlab.http_list(path, iterator=True, **kwargs)
159151
if TYPE_CHECKING:
160152
assert isinstance(data_list, RESTObjectList)
161153
manager = ProjectMergeRequestManager(

‎gitlab/v4/objects/repositories.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ def repository_tree(
6060
all: If True, return all the items, without pagination
6161
per_page: Number of items to retrieve per request
6262
page: ID of the page to return (starts with page 1)
63-
as_list: If set to False and no pagination option is
63+
iterator: If set to True and no pagination option is
6464
defined, return a generator instead of a list
6565
**kwargs: Extra options to send to the server (e.g. sudo)
6666
@@ -172,7 +172,7 @@ def repository_contributors(
172172
all: If True, return all the items, without pagination
173173
per_page: Number of items to retrieve per request
174174
page: ID of the page to return (starts with page 1)
175-
as_list: If set to False and no pagination option is
175+
iterator: If set to True and no pagination option is
176176
defined, return a generator instead of a list
177177
**kwargs: Extra options to send to the server (e.g. sudo)
178178

‎gitlab/v4/objects/runners.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ def all(self, scope: Optional[str] = None, **kwargs: Any) -> List[Runner]:
8181
all: If True, return all the items, without pagination
8282
per_page: Number of items to retrieve per request
8383
page: ID of the page to return (starts with page 1)
84-
as_list: If set to False and no pagination option is
84+
iterator: If set to True and no pagination option is
8585
defined, return a generator instead of a list
8686
**kwargs: Extra options to send to the server (e.g. sudo)
8787

‎gitlab/v4/objects/users.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -542,12 +542,12 @@ def list(self, **kwargs: Any) -> Union[RESTObjectList, List[RESTObject]]:
542542
all: If True, return all the items, without pagination
543543
per_page: Number of items to retrieve per request
544544
page: ID of the page to return (starts with page 1)
545-
as_list: If set to False and no pagination option is
545+
iterator: If set to True and no pagination option is
546546
defined, return a generator instead of a list
547547
**kwargs: Extra options to send to the server (e.g. sudo)
548548
549549
Returns:
550-
The list of objects, or a generator if `as_list` is False
550+
The list of objects, or a generator if `iterator` is True
551551
552552
Raises:
553553
GitlabAuthenticationError: If authentication is not correct

‎tests/functional/api/test_gitlab.py

+14-3
Original file line numberDiff line numberDiff line change
@@ -220,9 +220,20 @@ def test_list_all_true_nowarning(gl):
220220
assert len(items) > 20
221221

222222

223-
def test_list_as_list_false_nowarning(gl):
224-
"""Using `as_list=False` will disable the warning"""
223+
def test_list_iterator_true_nowarning(gl):
224+
"""Using `iterator=True` will disable the warning"""
225225
with warnings.catch_warnings(record=True) as caught_warnings:
226-
items = gl.gitlabciymls.list(as_list=False)
226+
items = gl.gitlabciymls.list(iterator=True)
227227
assert len(caught_warnings) == 0
228228
assert len(list(items)) > 20
229+
230+
231+
def test_list_as_list_false_warnings(gl):
232+
"""Using `as_list=False` will disable the UserWarning but cause a
233+
DeprecationWarning"""
234+
with warnings.catch_warnings(record=True) as caught_warnings:
235+
items = gl.gitlabciymls.list(as_list=False)
236+
assert len(caught_warnings) == 1
237+
for warning in caught_warnings:
238+
assert isinstance(warning.message, DeprecationWarning)
239+
assert len(list(items)) > 20

‎tests/functional/api/test_projects.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ def test_create_project(gl, user):
1515
sudo_project = gl.projects.create({"name": "sudo_project"}, sudo=user.id)
1616

1717
created = gl.projects.list()
18-
created_gen = gl.projects.list(as_list=False)
18+
created_gen = gl.projects.list(iterator=True)
1919
owned = gl.projects.list(owned=True)
2020

2121
assert admin_project in created and sudo_project in created

‎tests/unit/mixins/test_mixin_methods.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ class M(ListMixin, FakeManager):
107107

108108
# test RESTObjectList
109109
mgr = M(gl)
110-
obj_list = mgr.list(as_list=False)
110+
obj_list = mgr.list(iterator=True)
111111
assert isinstance(obj_list, base.RESTObjectList)
112112
for obj in obj_list:
113113
assert isinstance(obj, FakeObject)
@@ -138,7 +138,7 @@ class M(ListMixin, FakeManager):
138138
)
139139

140140
mgr = M(gl)
141-
obj_list = mgr.list(path="/others", as_list=False)
141+
obj_list = mgr.list(path="/others", iterator=True)
142142
assert isinstance(obj_list, base.RESTObjectList)
143143
obj = obj_list.next()
144144
assert obj.id == 42

0 commit comments

Comments
 (0)