You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@libcloud.apache.org by "jan-mue (via GitHub)" <gi...@apache.org> on 2023/01/25 16:57:00 UTC
[GitHub] [libcloud] jan-mue opened a new pull request, #1850: Fix list_nodes in the Azure arm driver
jan-mue opened a new pull request, #1850:
URL: https://github.com/apache/libcloud/pull/1850
## Fix the list_nodes function in the Azure ARM driver
### Description
The Azure API returns an attribute `nextLink` for pagination that is currently not used in the Azure ARM driver.
Documentation: https://learn.microsoft.com/en-us/rest/api/compute/virtual-machines/list-all?tabs=HTTP
This PR fixes the issue where not all VMs are returned for subscriptions with a lot of VMs (#1824)
### Status
Replace this: work in progress
### Checklist (tick everything that applies)
- [ ] [Code linting](http://libcloud.readthedocs.org/en/latest/development.html#code-style-guide) (required, can be done after the PR checks)
- [ ] Documentation
- [ ] [Tests](http://libcloud.readthedocs.org/en/latest/testing.html)
- [ ] [ICLA](http://libcloud.readthedocs.org/en/latest/development.html#contributing-bigger-changes) (required for bigger changes)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [libcloud] asfgit merged pull request #1850: Fix list_nodes in the Azure arm driver
Posted by "asfgit (via GitHub)" <gi...@apache.org>.
asfgit merged PR #1850:
URL: https://github.com/apache/libcloud/pull/1850
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [libcloud] codecov-commenter commented on pull request #1850: Fix list_nodes in the Azure arm driver
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1850:
URL: https://github.com/apache/libcloud/pull/1850#issuecomment-1658448326
## [Codecov](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#1850](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (efe4978) into [trunk](https://app.codecov.io/gh/apache/libcloud/commit/d163f9ba5938aff7d23dec72fa549a482f6836c7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (d163f9b) will **increase** coverage by `0.00%`.
> Report is 30 commits behind head on trunk.
> The diff coverage is `89.39%`.
[](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## trunk #1850 +/- ##
=======================================
Coverage 83.10% 83.11%
=======================================
Files 353 353
Lines 81269 81317 +48
Branches 8587 8592 +5
=======================================
+ Hits 67541 67588 +47
- Misses 10922 10924 +2
+ Partials 2806 2805 -1
```
| [Files Changed](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [libcloud/backup/drivers/dimensiondata.py](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bGliY2xvdWQvYmFja3VwL2RyaXZlcnMvZGltZW5zaW9uZGF0YS5weQ==) | `94.94% <ø> (ø)` | |
| [libcloud/common/aliyun.py](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bGliY2xvdWQvY29tbW9uL2FsaXl1bi5weQ==) | `74.44% <ø> (ø)` | |
| [libcloud/common/azure.py](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bGliY2xvdWQvY29tbW9uL2F6dXJlLnB5) | `86.30% <ø> (ø)` | |
| [libcloud/common/azure\_arm.py](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bGliY2xvdWQvY29tbW9uL2F6dXJlX2FybS5weQ==) | `73.01% <ø> (ø)` | |
| [libcloud/common/durabledns.py](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bGliY2xvdWQvY29tbW9uL2R1cmFibGVkbnMucHk=) | `99.24% <ø> (ø)` | |
| [libcloud/common/google.py](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bGliY2xvdWQvY29tbW9uL2dvb2dsZS5weQ==) | `71.35% <ø> (ø)` | |
| [libcloud/common/gridscale.py](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bGliY2xvdWQvY29tbW9uL2dyaWRzY2FsZS5weQ==) | `83.92% <ø> (ø)` | |
| [libcloud/common/kubernetes.py](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bGliY2xvdWQvY29tbW9uL2t1YmVybmV0ZXMucHk=) | `80.80% <ø> (ø)` | |
| [libcloud/common/nttcis.py](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bGliY2xvdWQvY29tbW9uL250dGNpcy5weQ==) | `73.58% <ø> (ø)` | |
| [libcloud/common/vultr.py](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-bGliY2xvdWQvY29tbW9uL3Z1bHRyLnB5) | `84.93% <ø> (ø)` | |
| ... and [69 more](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
------
[Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). Last update [d163f9b...efe4978](https://app.codecov.io/gh/apache/libcloud/pull/1850?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [libcloud] Kami commented on a diff in pull request #1850: Fix list_nodes in the Azure arm driver
Posted by "Kami (via GitHub)" <gi...@apache.org>.
Kami commented on code in PR #1850:
URL: https://github.com/apache/libcloud/pull/1850#discussion_r1279288354
##########
libcloud/compute/drivers/azure_arm.py:
##########
@@ -468,11 +468,17 @@ def list_nodes(self, ex_resource_group=None, ex_fetch_nic=True, ex_fetch_power_s
action = "/subscriptions/%s/providers/Microsoft.Compute/" "virtualMachines" % (
self.subscription_id
)
- r = self.connection.request(action, params={"api-version": VM_API_VERSION})
- return [
- self._to_node(n, fetch_nic=ex_fetch_nic, fetch_power_state=ex_fetch_power_state)
- for n in r.object["value"]
- ]
+ params = {"api-version": VM_API_VERSION}
+ nodes = []
+ while True:
Review Comment:
I think this code is far for now, but to be on the safe side and avoid possible infinite loop, I think it would be good to add additional guard here - e.g. bail out if we are still stuck in this loop for more than X seconds.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [libcloud] Kami commented on pull request #1850: Fix list_nodes in the Azure arm driver
Posted by "Kami (via GitHub)" <gi...@apache.org>.
Kami commented on PR #1850:
URL: https://github.com/apache/libcloud/pull/1850#issuecomment-1658361599
@jan-mue Thanks for adding a test case. I will review it shortly and if everything looks fine, merge changes into trunk.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [libcloud] Kami commented on pull request #1850: Fix list_nodes in the Azure arm driver
Posted by "Kami (via GitHub)" <gi...@apache.org>.
Kami commented on PR #1850:
URL: https://github.com/apache/libcloud/pull/1850#issuecomment-1407409093
Thanks for the contribution.
It would be great if you could also add a unit test for this change.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [libcloud] Kami commented on pull request #1850: Fix list_nodes in the Azure arm driver
Posted by "Kami (via GitHub)" <gi...@apache.org>.
Kami commented on PR #1850:
URL: https://github.com/apache/libcloud/pull/1850#issuecomment-1658364437
@jan-mue Can you please also sign an ICLA when you get a chance (https://libcloud.readthedocs.io/en/latest/development.html#contributing-bigger-changes)? Thanks.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [libcloud] Kami commented on pull request #1850: Fix list_nodes in the Azure arm driver
Posted by "Kami (via GitHub)" <gi...@apache.org>.
Kami commented on PR #1850:
URL: https://github.com/apache/libcloud/pull/1850#issuecomment-1658422834
I pushed a change so we don't end up in an infinite loop in case server returns a bad response, there is a bug in the code or similar - c49ecd0a4b8fe1e3820d8764f9aded21e5dee11c.
I picked that 60 seconds limit arbitrarily, but if needed or wanted, it can be changed (e.g. increased) - @jan-mue Can you please advise if 60 seconds is enough or the limit should be higher?
I imagine we may want a higher limit since depending on the method arguments, each ``_to_node()`` call will result in additional HTTP request(s).
For now, in case deadline is reached we simply return, but we may also want to throw or at least log a warning in such scenario.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [libcloud] Kami commented on pull request #1850: Fix list_nodes in the Azure arm driver
Posted by "Kami (via GitHub)" <gi...@apache.org>.
Kami commented on PR #1850:
URL: https://github.com/apache/libcloud/pull/1850#issuecomment-1658424921
EDIT: I went ahead and bumped that limit to 300 seconds for now. This way I can wrap up and merge this PR.
And then based on your future feedback @jan-mue, we can adjust this value.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [libcloud] jan-mue commented on pull request #1850: Fix list_nodes in the Azure arm driver
Posted by "jan-mue (via GitHub)" <gi...@apache.org>.
jan-mue commented on PR #1850:
URL: https://github.com/apache/libcloud/pull/1850#issuecomment-1507227012
@Kami sorry for the long delay, but I've added some unit tests now. Could you please have another look at the PR?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@libcloud.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org