Skip to content

Commit 33b4a70

Browse files
igorp-collaboraigorp-collabora
committed
feat(api)!: ListMixin.list typing overload
BREAKING CHANGE: The `http_list` method will no longer ignore the `iterator` argument if the `page` argument is not None. The `ListMixin.list()` will now return either list or iterator based on the `iterator` argument. This will make certain type narrowing checks redundant. Overload `ListMixin.list` method typing to return either `RESTObjectList` or `list` based on the `iterator` argument. By default then `iterator` is False return a list otherwise return an iterator. Provide 3 overloads: 1. `iterator: Literal[False]` - this is the default and returns a list. 2. `iterator: Literal[True]` - return an iterator. 3. `iterator: bool` - return either list or iterator. It is useful when the list function is being extended by another function that can also take either True or False for the `iterator` argument. Make `page` argument to `http_list` not override the `iterator` to make the function signatures more straight forward. This also makes it easier to unpack `**kwargs` as only `iterator` argument will control if a list or iterator is returned so the `**kwargs` can no longer have a hidden page argument.
1 parent beb2f24 commit 33b4a70

File tree

8 files changed

+134
-35
lines changed

8 files changed

+134
-35
lines changed

‎gitlab/client.py

+9-11
Original file line numberDiff line numberDiff line change
@@ -881,18 +881,16 @@ def http_list(
881881

882882
page = kwargs.get("page")
883883

884-
if iterator and page is not None:
885-
arg_used_message = f"iterator={iterator}"
886-
utils.warn(
887-
message=(
888-
f"`{arg_used_message}` and `page={page}` were both specified. "
889-
f"`{arg_used_message}` will be ignored and a `list` will be "
890-
f"returned."
891-
),
892-
category=UserWarning,
893-
)
884+
if iterator:
885+
if page is not None:
886+
utils.warn(
887+
message=(
888+
f"`{iterator=}` and `{page=}` were both specified. "
889+
f"`{page=}` will be ignored."
890+
),
891+
category=UserWarning,
892+
)
894893

895-
if iterator and page is None:
896894
# Generator requested
897895
return GitlabList(self, url, query_data, **kwargs)
898896

‎gitlab/mixins.py

+17-2
Original file line numberDiff line numberDiff line change
@@ -161,9 +161,24 @@ def refresh(self, **kwargs: Any) -> None:
161161
class ListMixin(HeadMixin[base.TObjCls]):
162162
_list_filters: tuple[str, ...] = ()
163163

164+
@overload
165+
def list(
166+
self, *, iterator: Literal[False] = False, **kwargs: Any
167+
) -> list[base.TObjCls]: ...
168+
169+
@overload
170+
def list(
171+
self, *, iterator: Literal[True] = True, **kwargs: Any
172+
) -> base.RESTObjectList[base.TObjCls]: ...
173+
174+
@overload
175+
def list(
176+
self, *, iterator: bool = False, **kwargs: Any
177+
) -> base.RESTObjectList[base.TObjCls] | list[base.TObjCls]: ...
178+
164179
@exc.on_http_error(exc.GitlabListError)
165180
def list(
166-
self, **kwargs: Any
181+
self, *, iterator: bool = False, **kwargs: Any
167182
) -> base.RESTObjectList[base.TObjCls] | list[base.TObjCls]:
168183
"""Retrieve a list of objects.
169184
@@ -203,7 +218,7 @@ def list(
203218
# Allow to overwrite the path, handy for custom listings
204219
path = data.pop("path", self.path)
205220

206-
obj = self.gitlab.http_list(path, **data)
221+
obj = self.gitlab.http_list(path, iterator=iterator, **data)
207222
if isinstance(obj, list):
208223
return [self._obj_cls(self, item, created_from_list=True) for item in obj]
209224
return base.RESTObjectList(self, self._obj_cls, obj)

‎gitlab/v4/cli.py

+4-7
Original file line numberDiff line numberDiff line change
@@ -133,12 +133,7 @@ def do_create(self) -> gitlab.base.RESTObject:
133133
cli.die("Impossible to create object", e)
134134
return result
135135

136-
def do_list(
137-
self,
138-
) -> (
139-
gitlab.base.RESTObjectList[gitlab.base.RESTObject]
140-
| list[gitlab.base.RESTObject]
141-
):
136+
def do_list(self) -> list[gitlab.base.RESTObject]:
142137
if TYPE_CHECKING:
143138
assert isinstance(self.mgr, gitlab.mixins.ListMixin)
144139
message_details = gitlab.utils.WarnMessageData(
@@ -150,7 +145,9 @@ def do_list(
150145
)
151146

152147
try:
153-
result = self.mgr.list(**self.args, message_details=message_details)
148+
result = self.mgr.list(
149+
**self.args, message_details=message_details, iterator=False
150+
)
154151
except Exception as e: # pragma: no cover, cli.die is unit-tested
155152
cli.die("Impossible to list objects", e)
156153
return result

‎gitlab/v4/objects/ldap.py

+20-3
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
from __future__ import annotations
22

3-
from typing import Any
3+
from typing import Any, Literal, overload
44

55
from gitlab import exceptions as exc
66
from gitlab.base import RESTManager, RESTObject, RESTObjectList
@@ -17,8 +17,25 @@ class LDAPGroupManager(RESTManager[LDAPGroup]):
1717
_obj_cls = LDAPGroup
1818
_list_filters = ("search", "provider")
1919

20+
@overload
21+
def list(
22+
self, *, iterator: Literal[False] = False, **kwargs: Any
23+
) -> list[LDAPGroup]: ...
24+
25+
@overload
26+
def list(
27+
self, *, iterator: Literal[True] = True, **kwargs: Any
28+
) -> RESTObjectList[LDAPGroup]: ...
29+
30+
@overload
31+
def list(
32+
self, *, iterator: bool = False, **kwargs: Any
33+
) -> list[LDAPGroup] | RESTObjectList[LDAPGroup]: ...
34+
2035
@exc.on_http_error(exc.GitlabListError)
21-
def list(self, **kwargs: Any) -> list[LDAPGroup] | RESTObjectList[LDAPGroup]:
36+
def list(
37+
self, *, iterator: bool = False, **kwargs: Any
38+
) -> list[LDAPGroup] | RESTObjectList[LDAPGroup]:
2239
"""Retrieve a list of objects.
2340
2441
Args:
@@ -45,7 +62,7 @@ def list(self, **kwargs: Any) -> list[LDAPGroup] | RESTObjectList[LDAPGroup]:
4562
else:
4663
path = self._path
4764

48-
obj = self.gitlab.http_list(path, **data)
65+
obj = self.gitlab.http_list(path, iterator=iterator, **data)
4966
if isinstance(obj, list):
5067
return [self._obj_cls(self, item) for item in obj]
5168
return RESTObjectList(self, self._obj_cls, obj)

‎gitlab/v4/objects/snippets.py

+61-6
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,25 @@ class SnippetManager(CRUDMixin[Snippet]):
108108
optional=("title", "files", "file_name", "content", "visibility", "description")
109109
)
110110

111+
@overload
112+
def list_public(
113+
self, *, iterator: Literal[False] = False, **kwargs: Any
114+
) -> list[Snippet]: ...
115+
116+
@overload
117+
def list_public(
118+
self, *, iterator: Literal[True] = True, **kwargs: Any
119+
) -> RESTObjectList[Snippet]: ...
120+
121+
@overload
122+
def list_public(
123+
self, *, iterator: bool = False, **kwargs: Any
124+
) -> RESTObjectList[Snippet] | list[Snippet]: ...
125+
111126
@cli.register_custom_action(cls_names="SnippetManager")
112-
def list_public(self, **kwargs: Any) -> RESTObjectList[Snippet] | list[Snippet]:
127+
def list_public(
128+
self, *, iterator: bool = False, **kwargs: Any
129+
) -> RESTObjectList[Snippet] | list[Snippet]:
113130
"""List all public snippets.
114131
115132
Args:
@@ -126,10 +143,27 @@ def list_public(self, **kwargs: Any) -> RESTObjectList[Snippet] | list[Snippet]:
126143
Returns:
127144
The list of snippets, or a generator if `iterator` is True
128145
"""
129-
return self.list(path="/snippets/public", **kwargs)
146+
return self.list(path="/snippets/public", iterator=iterator, **kwargs)
147+
148+
@overload
149+
def list_all(
150+
self, *, iterator: Literal[False] = False, **kwargs: Any
151+
) -> list[Snippet]: ...
152+
153+
@overload
154+
def list_all(
155+
self, *, iterator: Literal[True] = True, **kwargs: Any
156+
) -> RESTObjectList[Snippet]: ...
157+
158+
@overload
159+
def list_all(
160+
self, *, iterator: bool = False, **kwargs: Any
161+
) -> RESTObjectList[Snippet] | list[Snippet]: ...
130162

131163
@cli.register_custom_action(cls_names="SnippetManager")
132-
def list_all(self, **kwargs: Any) -> RESTObjectList[Snippet] | list[Snippet]:
164+
def list_all(
165+
self, *, iterator: bool = False, **kwargs: Any
166+
) -> RESTObjectList[Snippet] | list[Snippet]:
133167
"""List all snippets.
134168
135169
Args:
@@ -146,9 +180,30 @@ def list_all(self, **kwargs: Any) -> RESTObjectList[Snippet] | list[Snippet]:
146180
Returns:
147181
A generator for the snippets list
148182
"""
149-
return self.list(path="/snippets/all", **kwargs)
183+
return self.list(path="/snippets/all", iterator=iterator, **kwargs)
184+
185+
@overload
186+
def public(
187+
self,
188+
*,
189+
iterator: Literal[False] = False,
190+
page: int | None = None,
191+
**kwargs: Any,
192+
) -> list[Snippet]: ...
193+
194+
@overload
195+
def public(
196+
self, *, iterator: Literal[True] = True, **kwargs: Any
197+
) -> RESTObjectList[Snippet]: ...
198+
199+
@overload
200+
def public(
201+
self, *, iterator: bool = False, **kwargs: Any
202+
) -> RESTObjectList[Snippet] | list[Snippet]: ...
150203

151-
def public(self, **kwargs: Any) -> RESTObjectList[Snippet] | list[Snippet]:
204+
def public(
205+
self, *, iterator: bool = False, **kwargs: Any
206+
) -> RESTObjectList[Snippet] | list[Snippet]:
152207
"""List all public snippets.
153208
154209
Args:
@@ -172,7 +227,7 @@ def public(self, **kwargs: Any) -> RESTObjectList[Snippet] | list[Snippet]:
172227
),
173228
category=DeprecationWarning,
174229
)
175-
return self.list(path="/snippets/public", **kwargs)
230+
return self.list(path="/snippets/public", iterator=iterator, **kwargs)
176231

177232

178233
class ProjectSnippet(UserAgentDetailMixin, SaveMixin, ObjectDeleteMixin, RESTObject):

‎gitlab/v4/objects/users.py

+20-3
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
from __future__ import annotations
88

9-
from typing import Any, cast, Optional
9+
from typing import Any, cast, Literal, Optional, overload
1010

1111
import requests
1212

@@ -623,7 +623,24 @@ class UserProjectManager(ListMixin[UserProject], CreateMixin[UserProject]):
623623
"id_before",
624624
)
625625

626-
def list(self, **kwargs: Any) -> RESTObjectList[UserProject] | list[UserProject]:
626+
@overload
627+
def list(
628+
self, *, iterator: Literal[False] = False, **kwargs: Any
629+
) -> list[UserProject]: ...
630+
631+
@overload
632+
def list(
633+
self, *, iterator: Literal[True] = True, **kwargs: Any
634+
) -> RESTObjectList[UserProject]: ...
635+
636+
@overload
637+
def list(
638+
self, *, iterator: bool = False, **kwargs: Any
639+
) -> RESTObjectList[UserProject] | list[UserProject]: ...
640+
641+
def list(
642+
self, *, iterator: bool = False, **kwargs: Any
643+
) -> RESTObjectList[UserProject] | list[UserProject]:
627644
"""Retrieve a list of objects.
628645
629646
Args:
@@ -645,7 +662,7 @@ def list(self, **kwargs: Any) -> RESTObjectList[UserProject] | list[UserProject]
645662
path = f"/users/{self._parent.id}/projects"
646663
else:
647664
path = f"/users/{self._from_parent_attrs['user_id']}/projects"
648-
return super().list(path=path, **kwargs)
665+
return super().list(path=path, iterator=iterator, **kwargs)
649666

650667

651668
class StarredProject(RESTObject):

‎tests/functional/api/test_merge_requests.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ def test_merge_request_reset_approvals(gitlab_url, project):
177177
# Pause to let GL catch up (happens on hosted too, sometimes takes a while for server to be ready to merge)
178178
time.sleep(5)
179179

180-
mr = bot_project.mergerequests.list()[0] # type: ignore[index]
180+
mr = bot_project.mergerequests.list()[0]
181181

182182
assert mr.reset_approvals()
183183

‎tests/unit/test_gitlab_http_methods.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -567,8 +567,8 @@ def test_list_request_page_and_iterator(gl):
567567
UserWarning, match="`iterator=True` and `page=1` were both specified"
568568
):
569569
result = gl.http_list("/projects", iterator=True, page=1)
570-
assert isinstance(result, list)
571-
assert len(result) == 20
570+
assert isinstance(result, GitlabList)
571+
assert len(list(result)) == 20
572572
assert len(responses.calls) == 1
573573

574574

0 commit comments

Comments
 (0)