Skip to content

Commit 2d284d1

Browse files
EnricoMiEnricoMi
authored
Make Repository.compare().commits return paginated list (#2882)
1 parent 14af705 commit 2d284d1

File tree

9 files changed

+128
-13
lines changed

9 files changed

+128
-13
lines changed

‎doc/changes.rst‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,29 @@ Change log
44
Stable versions
55
~~~~~~~~~~~~~~~
66

7+
Version 2.2.0 ()
8+
-----------------------------------
9+
10+
Breaking Changes
11+
^^^^^^^^^^^^^^^^
12+
13+
* The ``github.Comparison.Comparison`` instance returned by ``Repository.compare`` provides a ``commits``
14+
property that used to return a ``list[github.Commit.Commit]``, which has now been changed
15+
to ``PaginatedList[github.Commit.Commit]``. This breaks user code that assumes a ``list``:
16+
17+
.. code-block:: python
18+
19+
commits = repo.compare("v0.6", "v0.7").commits
20+
no_of_commits = len(commits)
21+
22+
This will raise a ``TypeError: object of type 'PaginatedList' has no len()``, as the returned ``PaginatedList``
23+
does not support the ``len()`` method. Use the ``totalCount`` property instead:
24+
25+
.. code-block:: python
26+
27+
commits = repo.compare("v0.6", "v0.7").commits
28+
no_of_commits = commits.totalCount
29+
730
Version 2.1.1 (September 29, 2023)
831
-----------------------------------
932

‎github/Comparison.py‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import github.Commit
4141
import github.File
4242
from github.GithubObject import Attribute, CompletableGithubObject, NotSet
43+
from github.PaginatedList import PaginatedList
4344

4445

4546
class Comparison(CompletableGithubObject):
@@ -51,7 +52,6 @@ def _initAttributes(self) -> None:
5152
self._ahead_by: Attribute[int] = NotSet
5253
self._base_commit: Attribute[github.Commit.Commit] = NotSet
5354
self._behind_by: Attribute[int] = NotSet
54-
self._commits: Attribute[list[github.Commit.Commit]] = NotSet
5555
self._diff_url: Attribute[str] = NotSet
5656
self._files: Attribute[list[github.File.File]] = NotSet
5757
self._html_url: Attribute[str] = NotSet
@@ -80,10 +80,21 @@ def behind_by(self) -> int:
8080
self._completeIfNotSet(self._behind_by)
8181
return self._behind_by.value
8282

83+
# This should be a method, but this used to be a property and cannot be changed without breaking user code
84+
# TODO: remove @property on version 3
8385
@property
84-
def commits(self) -> list[github.Commit.Commit]:
85-
self._completeIfNotSet(self._commits)
86-
return self._commits.value
86+
def commits(self) -> PaginatedList[github.Commit.Commit]:
87+
return PaginatedList(
88+
github.Commit.Commit,
89+
self._requester,
90+
self.url,
91+
{},
92+
None,
93+
"commits",
94+
"total_commits",
95+
self.raw_data,
96+
self.raw_headers,
97+
)
8798

8899
@property
89100
def diff_url(self) -> str:
@@ -137,8 +148,6 @@ def _useAttributes(self, attributes: dict[str, Any]) -> None:
137148
self._base_commit = self._makeClassAttribute(github.Commit.Commit, attributes["base_commit"])
138149
if "behind_by" in attributes: # pragma no branch
139150
self._behind_by = self._makeIntAttribute(attributes["behind_by"])
140-
if "commits" in attributes: # pragma no branch
141-
self._commits = self._makeListOfClassesAttribute(github.Commit.Commit, attributes["commits"])
142151
if "diff_url" in attributes: # pragma no branch
143152
self._diff_url = self._makeStringAttribute(attributes["diff_url"])
144153
if "files" in attributes: # pragma no branch

‎github/EnterpriseConsumedLicenses.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ def get_users(self) -> PaginatedList[NamedEnterpriseUser]:
8787
url_parameters,
8888
None,
8989
"users",
90-
self.raw_data,
91-
self.raw_headers,
90+
firstData=self.raw_data,
91+
firstHeaders=self.raw_headers,
9292
)
9393

9494
def _useAttributes(self, attributes: Dict[str, Any]) -> None:

‎github/PaginatedList.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,7 @@ def __init__(
152152
firstParams: Any,
153153
headers: Optional[Dict[str, str]] = None,
154154
list_item: str = "items",
155+
total_count_item: str = "total_count",
155156
firstData: Optional[Any] = None,
156157
firstHeaders: Optional[Dict[str, Union[str, int]]] = None,
157158
attributesTransformer: Optional[Callable[[Dict[str, Any]], Dict[str, Any]]] = None,
@@ -164,6 +165,7 @@ def __init__(
164165
self.__nextParams = firstParams or {}
165166
self.__headers = headers
166167
self.__list_item = list_item
168+
self.__total_count_item = total_count_item
167169
if self.__requester.per_page != 30:
168170
self.__nextParams["per_page"] = self.__requester.per_page
169171
self._reversed = False
@@ -255,7 +257,7 @@ def _getPage(self, data: Any, headers: Dict[str, Any]) -> List[T]:
255257
self.__nextUrl = links["next"]
256258
self.__nextParams = None
257259
if self.__list_item in data:
258-
self.__totalCount = data.get("total_count")
260+
self.__totalCount = data.get(self.__total_count_item)
259261
data = data[self.__list_item]
260262
content = [
261263
self.__contentClass(self.__requester, headers, self._transformAttributes(element), completed=False)

‎github/Repository.py‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1105,7 +1105,7 @@ def remove_invitation(self, invite_id: int) -> None:
11051105

11061106
def compare(self, base: str, head: str) -> Comparison:
11071107
"""
1108-
:calls: `GET /repos/{owner}/{repo}/compare/{base...:head} <https://docs.github.com/en/rest/reference/repos#commits>`_
1108+
:calls: `GET /repos/{owner}/{repo}/compare/{base...:head} <https://docs.github.com/en/rest/commits/commits#compare-two-commits>`_
11091109
:param base: string
11101110
:param head: string
11111111
:rtype: :class:`github.Comparison.Comparison`
@@ -1114,7 +1114,11 @@ def compare(self, base: str, head: str) -> Comparison:
11141114
assert isinstance(head, str), head
11151115
base = urllib.parse.quote(base)
11161116
head = urllib.parse.quote(head)
1117-
headers, data = self._requester.requestJsonAndCheck("GET", f"{self.url}/compare/{base}...{head}")
1117+
# the compare API has a per_page default of 250, which is different to Consts.DEFAULT_PER_PAGE
1118+
per_page = self._requester.per_page if self._requester.per_page != Consts.DEFAULT_PER_PAGE else 250
1119+
# only with page=1 we get the pagination headers for the commits element
1120+
params = {"page": 1, "per_page": per_page}
1121+
headers, data = self._requester.requestJsonAndCheck("GET", f"{self.url}/compare/{base}...{head}", params)
11181122
return github.Comparison.Comparison(self._requester, headers, data, completed=True)
11191123

11201124
def create_autolink(

‎tests/ReplayData/ExposeAllAttributes.testAllClasses.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ https
299299
GET
300300
api.github.com
301301
None
302-
/repos/jacquev6/PyGithub/compare/master...develop
302+
/repos/jacquev6/PyGithub/compare/master...develop?page=1&per_page=250
303303
{'Authorization': 'Basic login_and_password_removed', 'User-Agent': 'PyGithub/Python'}
304304
None
305305
200

‎tests/ReplayData/Repository.testCompare.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ https
22
GET
33
api.github.com
44
None
5-
/repos/jacquev6/PyGithub/compare/v0.6...v0.7
5+
/repos/jacquev6/PyGithub/compare/v0.6...v0.7?page=1&per_page=250
66
{'Authorization': 'Basic login_and_password_removed', 'User-Agent': 'PyGithub/Python'}
77
None
88
200

‎tests/ReplayData/Repository.testCompareCommitPagination.txt‎

Lines changed: 43 additions & 0 deletions
Large diffs are not rendered by default.

‎tests/Repository.py‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,40 @@ def testCompare(self):
704704
],
705705
)
706706

707+
def testCompareCommitPagination(self):
708+
gh = github.Github(
709+
auth=self.oauth_token,
710+
per_page=4,
711+
retry=self.retry,
712+
pool_size=self.pool_size,
713+
seconds_between_requests=self.seconds_between_requests,
714+
seconds_between_writes=self.seconds_between_writes,
715+
)
716+
repo = gh.get_repo("PyGithub/PyGithub")
717+
comparison = repo.compare("v1.54", "v1.54.1")
718+
self.assertEqual(comparison.status, "ahead")
719+
self.assertEqual(comparison.ahead_by, 10)
720+
self.assertEqual(comparison.behind_by, 0)
721+
self.assertEqual(comparison.total_commits, 10)
722+
self.assertEqual(len(comparison.files), 228)
723+
self.assertEqual(comparison.commits.totalCount, 10)
724+
self.assertListKeyEqual(
725+
comparison.commits,
726+
lambda c: c.sha,
727+
[
728+
"fab682a5ccfc275c31ec37f1f541254c7bd780f3",
729+
"9ee3afb1716c559a0b3b44e097c05f4b14ae2ab8",
730+
"a806b5233f6423e0f8dacc4d04b6d81a72689bed",
731+
"63e4fae997a9a5dc8c2b56907c87c565537bb28f",
732+
"82c349ce3e1c556531110753831b3133334c19b7",
733+
"2432cffd3b2f1a8e0b6b96d69b3dd4ded148a9f7",
734+
"e113e37de1ec687c68337d777f3629251b35ab28",
735+
"f299699ccd75910593d5ddf7cc6212f70c5c28b1",
736+
"31a1c007808a4205bdae691385d2627c561e69ed",
737+
"34d097ce473601624722b90fc5d0396011dd3acb",
738+
],
739+
)
740+
707741
def testGetComments(self):
708742
self.assertListKeyEqual(
709743
self.repo.get_comments(),

0 commit comments

Comments
 (0)