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

Need to call page when using GCEList for pagination to work #1095

Closed
wants to merge 1 commit into from

Conversation

sayap
Copy link
Contributor

@sayap sayap commented Aug 28, 2017

Otherwise, the if self.gce_params: condition will evaluate to false as the dict remains empty.

This is a follow-up fix to #939

@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 88.978% when pulling f2395f3 on KMK-ONLINE:gce-500-images into 7c84193 on apache:trunk.

@tonybaloney
Copy link
Contributor

Thanks for the contribution @sayap please can you resolve the linting error https://travis-ci.org/apache/libcloud/jobs/269134539

Otherwise, the `if self.gce_params:` condition will evaluate to false
as the dict remains empty.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.003%) to 88.978% when pulling 8bb7e0c on KMK-ONLINE:gce-500-images into 84735c8 on apache:trunk.

@sayap
Copy link
Contributor Author

sayap commented Aug 29, 2017

@tonybaloney I have got rid of the linting error with slightly better variable naming :)

@pquentin
Copy link
Contributor

pquentin commented Sep 4, 2017

Note that GCEList defines an __iter__ method, so it would be simpler to write for image in project_images_list: and remove the for image in page:.

But your fix works too and is more explicit about the pagination! 👍

@sayap
Copy link
Contributor Author

sayap commented Sep 5, 2017

@pquentin Actually, pagination only works after calling the page method.

We need this:

self.params['maxResults'] = max_results

so that this evaluates to true and we get pagination:

if self.gce_params:

I can't speak for the original author, but it seems like that's how it was designed to work in 1ed5020

@pquentin
Copy link
Contributor

pquentin commented Sep 5, 2017

Oh, you're right, sorry! Got confused by the return self hack. I now understand that __iter__ return sublists.

@pquentin
Copy link
Contributor

@tonybaloney @Kami this pull request is ready to be merged.

@asfgit asfgit closed this in 5892fa1 Sep 26, 2017
@pquentin
Copy link
Contributor

@sayap Thank you for your contribution! It has just been merged in trunk.

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

4 participants