Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

python-gitlab Issue #63 - implement pagination for list() #64

Merged
merged 3 commits into from
Aug 21, 2015

Conversation

jantman
Copy link
Contributor

@jantman jantman commented Jul 28, 2015

This implements iteration/pagination of list() results for #63

@@ -26,6 +26,9 @@
import requests
import six

import logging
logger = logging.getLogger(__name__)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you remove the logging code for this change? I'm not against adding some logging but I'd rather do it in another patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@jantman
Copy link
Contributor Author

jantman commented Aug 5, 2015

I've removed the logging code as requested, and added unit tests for the current implementation of just the next link. However, I think I've hit a bit of a roadblock...

I have nothing against giving a choice to the user, but I'd much rather prefer the iteration to be the default, with disabling it an option. I guess this is personal preference, but I find it very annoying when an API client presents a method like Users() that only retrieves partial results without making that extremely clear.

I looked into turning this into a generator, but as far as I can tell, I'd need to drastically change how arguments and return values are passed around to get away from the current recursive pattern.

gpocentek pushed a commit that referenced this pull request Aug 21, 2015
python-gitlab Issue #63 - implement pagination for list()
@gpocentek gpocentek merged commit 24d5035 into python-gitlab:master Aug 21, 2015
@gpocentek
Copy link
Contributor

Thanks Jason.

I've merged your patch as is but I'll probably restore the default behavior to avoid surprises on updates (people will not expect to suddenly retrieve all the items).

@jantman
Copy link
Contributor Author

jantman commented Aug 21, 2015

Ok, thanks! I'll keep an eye out for it, and update my code to use the pagination once it's released.

@gpocentek gpocentek mentioned this pull request Sep 19, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants