You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@libcloud.apache.org by GitBox <gi...@apache.org> on 2020/01/14 06:27:26 UTC
[GitHub] [libcloud] Kami opened a new pull request #1409: [WIP] Fix
pagination in the "list_nodes()" method in the GCE driver
Kami opened a new pull request #1409: [WIP] Fix pagination in the "list_nodes()" method in the GCE driver
URL: https://github.com/apache/libcloud/pull/1409
This pull request is an attempt to try to fix an issue reported in #515.
### Background / Problem Description
Currently ``list_nodes()`` method only returns up to 500 nodes (which is a page size value we use with our list requests and also a max page size limit set by Google).
This is true either when we are requesting nodes for all the zones (aggregated request) or when requesting nodes for a single zone.
### Proposed Fix
This pull request fixes that by utilizing ``request_aggregated_items`` method in both scenarios (when we are requesting nodes in all the zones and also when requesting nodes for a single zone).
I tested the change by setting ``maxResults`` in the ``request_aggregated_items`` method to ``1`` and verified that all the pages are correctly retrieved since I don't have an account with more than 500 instances to test it out.
---
On a somewhat related note - unless I'm missing something, it appears the same issue may also exist with other "list" API endpoints when retrieving items for a specific zone and that response contains more then max page size number of items (since we don't follow through pagination when requesting items for a single zone).
As you can see in the diff, this change also allowed me to simplify ``list_nodes()`` method. If my assertion / observation above is correct, this would mean we can also simplify other list methods.
@erjohnso Please correct me if I'm wrong or missing something.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] codecov-io commented on issue #1409: [WIP] Fix
pagination in the "list_nodes()" method in the GCE driver
Posted by GitBox <gi...@apache.org>.
codecov-io commented on issue #1409: [WIP] Fix pagination in the "list_nodes()" method in the GCE driver
URL: https://github.com/apache/libcloud/pull/1409#issuecomment-574025055
# [Codecov](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=h1) Report
> Merging [#1409](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=desc) into [trunk](https://codecov.io/gh/apache/libcloud/commit/17f963639fa4c2a59f63bb4fb99c9afc4a5b2d63?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## trunk #1409 +/- ##
==========================================
+ Coverage 86.46% 86.46% +<.01%
==========================================
Files 366 366
Lines 76773 76770 -3
Branches 7529 7528 -1
==========================================
- Hits 66378 66377 -1
+ Misses 7527 7525 -2
Partials 2868 2868
```
| [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libcloud/compute/drivers/gce.py](https://codecov.io/gh/apache/libcloud/pull/1409/diff?src=pr&el=tree#diff-bGliY2xvdWQvY29tcHV0ZS9kcml2ZXJzL2djZS5weQ==) | `76.9% <100%> (+0.04%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=footer). Last update [17f9636...c8a472f](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] codecov-io edited a comment on issue #1409: [WIP] Fix
pagination in the "list_nodes()" method in the GCE driver
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #1409: [WIP] Fix pagination in the "list_nodes()" method in the GCE driver
URL: https://github.com/apache/libcloud/pull/1409#issuecomment-574025055
# [Codecov](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=h1) Report
> Merging [#1409](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=desc) into [trunk](https://codecov.io/gh/apache/libcloud/commit/17f963639fa4c2a59f63bb4fb99c9afc4a5b2d63?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `78.94%`.
[](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## trunk #1409 +/- ##
==========================================
+ Coverage 86.46% 86.46% +<.01%
==========================================
Files 366 366
Lines 76773 76771 -2
Branches 7529 7528 -1
==========================================
- Hits 66378 66377 -1
+ Misses 7527 7526 -1
Partials 2868 2868
```
| [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libcloud/compute/drivers/gce.py](https://codecov.io/gh/apache/libcloud/pull/1409/diff?src=pr&el=tree#diff-bGliY2xvdWQvY29tcHV0ZS9kcml2ZXJzL2djZS5weQ==) | `76.88% <78.94%> (+0.01%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=footer). Last update [17f9636...d0a62fb](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] codecov-io edited a comment on issue #1409: [WIP] Fix
pagination in the "list_nodes()" method in the GCE driver
Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on issue #1409: [WIP] Fix pagination in the "list_nodes()" method in the GCE driver
URL: https://github.com/apache/libcloud/pull/1409#issuecomment-574025055
# [Codecov](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=h1) Report
> Merging [#1409](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=desc) into [trunk](https://codecov.io/gh/apache/libcloud/commit/17f963639fa4c2a59f63bb4fb99c9afc4a5b2d63?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.
[](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## trunk #1409 +/- ##
==========================================
+ Coverage 86.46% 86.46% +<.01%
==========================================
Files 366 366
Lines 76773 76770 -3
Branches 7529 7528 -1
==========================================
- Hits 66378 66377 -1
+ Misses 7527 7525 -2
Partials 2868 2868
```
| [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [libcloud/compute/drivers/gce.py](https://codecov.io/gh/apache/libcloud/pull/1409/diff?src=pr&el=tree#diff-bGliY2xvdWQvY29tcHV0ZS9kcml2ZXJzL2djZS5weQ==) | `76.9% <100%> (+0.04%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=footer). Last update [17f9636...c8a472f](https://codecov.io/gh/apache/libcloud/pull/1409?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] Kami commented on a change in pull request #1409: [WIP]
Fix pagination in the "list_nodes()" method in the GCE driver
Posted by GitBox <gi...@apache.org>.
Kami commented on a change in pull request #1409: [WIP] Fix pagination in the "list_nodes()" method in the GCE driver
URL: https://github.com/apache/libcloud/pull/1409#discussion_r366173417
##########
File path: libcloud/compute/drivers/gce.py
##########
@@ -2569,50 +2586,35 @@ def list_nodes(self, ex_zone=None, ex_use_disk_cache=True):
:return: List of Node objects
:rtype: ``list`` of :class:`Node`
"""
- list_nodes = []
zone = self._set_zone(ex_zone)
- if zone is None:
- request = '/aggregated/instances'
- else:
- request = '/zones/%s/instances' % (zone.name)
- response = self.connection.request(request, method='GET').object
+ response = self.connection.request_aggregated_items('instances',
+ zone=zone)
+
+ if not response.get('items', []):
Review comment:
This return early change is purely for style purpose so we can make the code a bit easier to read and get rid of one level of indentation on "happy" code path when response contains results.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] Kami commented on issue #1409: Fix pagination in the
"list_nodes()" method in the GCE driver
Posted by GitBox <gi...@apache.org>.
Kami commented on issue #1409: Fix pagination in the "list_nodes()" method in the GCE driver
URL: https://github.com/apache/libcloud/pull/1409#issuecomment-578040627
I'll merge this since I'm working on v3.0.0rc1 release so that change can be included as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] Kami merged pull request #1409: Fix pagination in the
"list_nodes()" method in the GCE driver
Posted by GitBox <gi...@apache.org>.
Kami merged pull request #1409: Fix pagination in the "list_nodes()" method in the GCE driver
URL: https://github.com/apache/libcloud/pull/1409
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [libcloud] Kami commented on a change in pull request #1409: [WIP]
Fix pagination in the "list_nodes()" method in the GCE driver
Posted by GitBox <gi...@apache.org>.
Kami commented on a change in pull request #1409: [WIP] Fix pagination in the "list_nodes()" method in the GCE driver
URL: https://github.com/apache/libcloud/pull/1409#discussion_r366169690
##########
File path: libcloud/compute/drivers/gce.py
##########
@@ -159,6 +166,16 @@ def request_aggregated_items(self, api_name):
self.gce_params = params
response = self.request(request_path, method='GET').object
if 'items' in response:
+ if zone:
+ # Special case when we are handling pagination for a
+ # specific zone
+ items = response['items']
+ response['items'] = {}
+ response['items'] = {
+ 'zones/%s' % (zone): {
Review comment:
In aggregated request response, this key will contain the zone name (e.g. ``zones/europe-west1-b``), but the actual key is not used anywhere so the name is irrelevant and could really contain any value (it's just the structure of the actual value that matters).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services