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

max parameter on list() methods doesn't work #7

Closed
alfdelgado opened this issue Sep 14, 2016 · 5 comments
Closed

max parameter on list() methods doesn't work #7

alfdelgado opened this issue Sep 14, 2016 · 5 comments
Labels

Comments

@alfdelgado
Copy link

The max parameter does not actually limit the number of items retrieved.

The reason for this seems to be lines 79 through 90 in restsession.py

After calling requests.get() [line 77], the response is checked and yielded [lines 81 and 82]. But then the next page is requested and assigned to the response variable [lines 84 through 87], and the while loop [line 79] reprocesses the variable.

A simple solution would be to remove the loop altogether, leaving just lines 80 through 82; but I don't know if the paged behaviour is needed by other methods.

@alfdelgado
Copy link
Author

The simple solution I suggested will not do, as the backend API may limit the number of returned items. Either an if on the max parameter or a counter will be needed.

@alfdelgado
Copy link
Author

The attached patch adds a section to get_pages to handle requests where a maximum number of items has been requested explicitly.

restsession.patch.txt

@cmlccie
Copy link
Collaborator

cmlccie commented Sep 15, 2016

Alf,

Thank you for working on this today, and thank you for supplying a possible patch!

However, the 'max' parameter is there to control the number of items returned per page, not to indicate that only that many items should be returned.

From the developer documentation:
"When requesting a list of resources the max query parameter may be used to control the number of items returned per page."

So, simply saying that if the 'max' parameter is supplied - disable paging, doesn't represent the intent of the parameter.

That said, something is clearly going on here with the API calls and the returned 'link' addresses. If no 'max' parameter is supplied max = None, then we do not even send the max parameter as part of the request (there is an explicit if statement checking for this).

@cmlccie
Copy link
Collaborator

cmlccie commented Sep 15, 2016

As you identified today, and I just confirmed myself, this is an issue with the Cisco Spark cloud returning an invalid URL (contains 'max=null') in the 'next' Link header of paged responses.

I have raised this with them, hopefully they can get it corrected shortly. If not, we can patch this by parsing their returned URLs and removing the errant 'max=null' parameter.

@cmlccie cmlccie added the bug label Sep 15, 2016
cmlccie added a commit that referenced this issue Sep 17, 2016
@cmlccie
Copy link
Collaborator

cmlccie commented Sep 17, 2016

I have added a patch to workaround this issue, while Cisco works to fix the bug in the API. It is fixed in v0.1.2, which has been posted to PyPI.

@cmlccie cmlccie closed this as completed Sep 17, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants