NotificationsYou must be signed in to change notification settings
Fork676
Star2.4k

fix: handle situation where gitlab.com does not return values#1773

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

nejch merged 1 commit intomainfromjlvillal/pagination

Dec 31, 2021

Merged

fix: handle situation where gitlab.com does not return values#1773

nejch merged 1 commit intomainfromjlvillal/pagination

Dec 31, 2021

Conversation

Copy link

Member

JohnVillalovos commentedDec 22, 2021

If a query returns more than 10,000 records than the following values
are NOT returned:
x.total_pages
x.total

Modify the code to allow no value to be set for these values. If there
is not a value returned the functions will now return None.

https://docs.gitlab.com/ee/user/gitlab_com/index.html#pagination-response-headers

Closes#1686

JohnVillalovos requested a review fromnejch

December 22, 2021 22:14

Copy link

codecov-commenter commentedDec 22, 2021•
edited
Loading

Codecov Report

Merging#1773 (cb824a4) intomain (501f9a1) willincrease coverage by0.00%.
The diff coverage is100.00%.

@@           Coverage Diff           @@##             main    #1773   +/-   ##=======================================  Coverage   91.96%   91.97%           =======================================  Files          76       76             Lines        4754     4758    +4     =======================================+ Hits         4372     4376    +4  Misses        382      382

Flag	Coverage Δ
cli_func_v4	`81.21% <62.50%> (-0.07%)`	⬇️
py_func_v4	`80.62% <62.50%> (-0.07%)`	⬇️
unit	`83.18% <100.00%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown.Click here to find out more.

Impacted Files	Coverage Δ
gitlab/base.py	`90.22% <100.00%> (ø)`
gitlab/client.py	`90.10% <100.00%> (+0.10%)`	⬆️

JohnVillalovos force-pushed thejlvillal/pagination branch 2 times, most recently fromca2e8e0 to971a617Compare

December 22, 2021 22:38

nejch reviewed

Dec 25, 2021

View reviewed changes

docs/api-usage.rst OutdatedShow resolvedHide resolved

gitlab/client.py OutdatedShow resolvedHide resolved

JohnVillalovos force-pushed thejlvillal/pagination branch from971a617 to4690dc2Compare

December 25, 2021 17:27

Copy link

Member

nejch commentedDec 28, 2021

LGTM I'd just like to reproduce#1686 in some tests first with an xfail (e.g. I'll see if I can get responses to match the presence/absence of those headers). Are you ok to wait and rebase@JohnVillalovos (unless you want to take a look at it yourself but you already have a few PRs :D)

Copy link

MemberAuthor

JohnVillalovos commentedDec 28, 2021

LGTM I'd just like to reproduce#1686 in some tests first with an xfail (e.g. I'll see if I can get responses to match the presence/absence of those headers). Are you ok to wait and rebase@JohnVillalovos (unless you want to take a look at it yourself but you already have a few PRs :D)

Works for me. Thanks!

Copy link

Member

nejch commentedDec 30, 2021

I have#1788 ready for this and I also saw I was being overly pedantic as we're already doing the same with one-liners 😅 :

python-gitlab/gitlab/client.py

Lines 945 to 959 inc764bee

	@property
	defprev_page(self)->Optional[int]:
	"""The previous page number.

	If None, the current page is the first.
	"""
	returnint(self._prev_page)ifself._prev_pageelseNone

	@property
	defnext_page(self)->Optional[int]:
	"""The next page number.

	If None, the current page is the last.
	"""
	returnint(self._next_page)ifself._next_pageelseNone

JohnVillalovos force-pushed thejlvillal/pagination branch 2 times, most recently from4a17937 toc308c30Compare

December 30, 2021 19:32

JohnVillalovos requested a review fromnejch

December 30, 2021 19:33

JohnVillalovos force-pushed thejlvillal/pagination branch fromc308c30 to4947901Compare

December 30, 2021 19:34

Copy link

MemberAuthor

JohnVillalovos commentedDec 30, 2021

I have#1788 ready for this and I also saw I was being overly pedantic as we're already doing the same with one-liners 😅 :

Thanks. I reviewed and merged#1788. Then rebased this PR. Was surprised that if anxfail marked test passes it doesn't cause things to fail 😕 Removed thexfail, and it is working as expected now.

fix: handle situation where GitLab does not return values

cb824a4

If a query returns more than 10,000 records than the following valuesare NOT returned:  x.total_pages  x.totalModify the code to allow no value to be set for these values. If thereis not a value returned the functions will now return None.Update unit test so no longer `xfail`https://docs.gitlab.com/ee/user/gitlab_com/index.html#pagination-response-headersCloses#1686

JohnVillalovos force-pushed thejlvillal/pagination branch from4947901 tocb824a4Compare

December 30, 2021 20:34

Copy link

MemberAuthor

JohnVillalovos commentedDec 30, 2021

Was surprised that if anxfail marked test passes it doesn't cause things to fail 😕

I figured that out and addedxfail_strict to thepytest configuration. Now if anxfail test passes it will cause the test run to fail.

nejch merged commita3eafab intomain

Dec 31, 2021

nejch deleted the jlvillal/pagination branch

December 31, 2021 20:50

This was referencedJan 4, 2022

Handle pagination of results with more than 10,000 results gracefully#1506

Closed

Regression on calling len() on GitlabList objects since switching to keyset pagination by default#1114

Closed

Labels

None yet

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: handle situation where gitlab.com does not return values#1773

fix: handle situation where gitlab.com does not return values#1773

Uh oh!