No support for paginated responses?
Closed this issue · 17 comments
If any API call returns more than 25 results, it is paginated by DigitalOcean.
The wrapper responds by truncating and returning just the first 25 results.
I can work around this by pulling tricks like repeatedly using API functions with params={'page': str(n)} in order to get large result sets, but I'd prefer not to have to...
There is support for pages as you can see here:
https://github.com/koalalorenzo/python-digitalocean/blob/master/digitalocean/Manager.py#L31
But it is implemented on specific calls only. Which API or Class are you using? Maybe it was not implemented there.
Oh, you're right. I was fetching a list of DNS records.
I will probably implement it during the weekend on the missing classes!
Hi! I have 750 droplets, but get_all_droplets() method only shows 200. I just comented lines below and it worked.
if kwargs.get('type') == GET:
else:
return data
In the documentation (here: https://developers.digitalocean.com/documentation/v2/ ) some API are allowed to return a maximum of 200 elements. Probably the pagination system should fix that
@evandroalb: can you link the source code/lines?
Hi there. Same problem here - get_all_droplets returns only 200 results on version 1.9. It was working OK on version 1.8
Here is a link to the piece of code @evandroalb was walking about
https://github.com/koalalorenzo/python-digitalocean/blob/master/digitalocean/Manager.py#L40-L43
Introduced by this change
cf12fc1
Honestly, I had literally no time to check that :-(
If somebody has time to contribute and fix this it will be appreciated
Reviewing this, it seems like cf12fc1 should be reverted, and maybe we could document Manager.get_data() a bit better. Looking at the bug (#140) the change was supposed to fix, it seems like they were just using the wrong function. They try to create an SSH key with:
params = {'public_key': 'AAAAkey', 'name': 'new_key'}
ssh_key = self.manager.get_data(url='account/keys/', type='POST', params=params)
rather than just using SSHKey.create
Is that supposed to be supported? If so, the other approach is to make sure all the Manager functions explicitly pass the type=GET kwarg. Currently none of them do. It is just assumed. That's why when we get to the if kwargs.get('type') == GET nothing ends up using __deal_with_pagination
Yes, @andrewsomething is right, I am "reverting" that commit and I will probably rewrite a piece of it in order to avoid the problem that caused #140
unpaged_data = self.__deal_with_pagination(args[0], data, params)
About the SSHkey.create I do believe that we should follow always and only what the official documentation of the API is saying. As far as I remember, SSHKey.create should be the way, but if the API are allowing this, we should support it as well.
Sadly I will not be able to verify this, because I don't have 750 Droplets available. I will be more than happy to get some help for this :D
Is anybody testing this manually? Help is needed to verify this on the branch. Thanks
There was a slight bug in the change. Here is how I modified it in order to work:
if 'meta' in data and 'total' in data['meta']:
if data['meta']['total'] > params['per_page']:
return self.__deal_with_pagination(args[0], data, params)
else:
return data
else:
return data
Tested the above change and it works nicely both with paginated and unpaginated responses.
@goodcode is the change that I have just applied, what you intended?
Yes, that's exactly it. Sorry, I wasn't able to create a PR myself.
Pull request created! Waiting for some final feedback. I will merge it before the weekend 👍
That's great, thanks! Are you planning a new release soon?
Yes, I was planning to test and release it this evening (CET) but since this was tested as well, I will verify and release everything during the weekend.
Sounds great!