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 2022/08/06 00:07:42 UTC
[GitHub] [libcloud] palashgandhi opened a new pull request, #1736: azure-arm: Add support to create disks in specific availability zones
palashgandhi opened a new pull request, #1736:
URL: https://github.com/apache/libcloud/pull/1736
## azure-arm: Add support to create disks in specific availability zones
### Description
Azure ARM's compute driver does not provide the ability to pass zones when creating a new volume. This change adds a new parameter that allows passing zones as a list of strings. I have also merged the disk API versions because the `zones` parameter was only supported in newer API versions. I do not fully understand the impact of bumping the API versions, but if tests pass, I think it should be fine.
### Status
- done, ready for review
### Checklist (tick everything that applies)
- [x] [Code linting](http://libcloud.readthedocs.org/en/latest/development.html#code-style-guide) (required, can be done after the PR checks)
- [ ] Documentation
- [x] [Tests](http://libcloud.readthedocs.org/en/latest/testing.html)
- [x] [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] pgandhi-delphix commented on pull request #1736: azure-arm: Add support to create disks in specific availability zones
Posted by GitBox <gi...@apache.org>.
pgandhi-delphix commented on PR #1736:
URL: https://github.com/apache/libcloud/pull/1736#issuecomment-1212408790
@Kami when you get a chance, can you please take a look at this change? Thanks in advance.
--
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 #1736: azure-arm: Add support to create disks in specific availability zones
Posted by GitBox <gi...@apache.org>.
Kami commented on code in PR #1736:
URL: https://github.com/apache/libcloud/pull/1736#discussion_r961916040
##########
libcloud/compute/drivers/azure_arm.py:
##########
@@ -917,8 +914,9 @@ def create_volume(
location=None,
snapshot=None,
ex_resource_group=None,
- ex_account_type=None,
Review Comment:
So this option has been deprecated / removed?
--
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 #1736: azure-arm: Add support to create disks in specific availability zones
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1736:
URL: https://github.com/apache/libcloud/pull/1736#issuecomment-1207109335
# [Codecov](https://codecov.io/gh/apache/libcloud/pull/1736?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1736](https://codecov.io/gh/apache/libcloud/pull/1736?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3693f21) into [trunk](https://codecov.io/gh/apache/libcloud/commit/26f07ac1a28b35953b84c592d6476bf65e5aacf7?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (26f07ac) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/libcloud/pull/1736?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## trunk #1736 +/- ##
=======================================
Coverage 83.29% 83.29%
=======================================
Files 400 400
Lines 87770 87772 +2
Branches 9331 9332 +1
=======================================
+ Hits 73104 73106 +2
Misses 11486 11486
Partials 3180 3180
```
| [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1736?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [libcloud/compute/drivers/azure\_arm.py](https://codecov.io/gh/apache/libcloud/pull/1736/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGliY2xvdWQvY29tcHV0ZS9kcml2ZXJzL2F6dXJlX2FybS5weQ==) | `57.81% <100.00%> (+0.06%)` | :arrow_up: |
| [libcloud/test/compute/test\_azure\_arm.py](https://codecov.io/gh/apache/libcloud/pull/1736/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bGliY2xvdWQvdGVzdC9jb21wdXRlL3Rlc3RfYXp1cmVfYXJtLnB5) | `98.81% <100.00%> (+<0.01%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1736?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **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=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/libcloud/pull/1736?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [26f07ac...3693f21](https://codecov.io/gh/apache/libcloud/pull/1736?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). 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=The+Apache+Software+Foundation).
--
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 #1736: azure-arm: Add support to create disks in specific availability zones
Posted by GitBox <gi...@apache.org>.
Kami commented on PR #1736:
URL: https://github.com/apache/libcloud/pull/1736#issuecomment-1235617591
@pgandhi-delphix Sorry for the delay, I will look into it this weekend.
--
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 #1736: azure-arm: Add support to create disks in specific availability zones
Posted by GitBox <gi...@apache.org>.
Kami commented on PR #1736:
URL: https://github.com/apache/libcloud/pull/1736#issuecomment-1235829430
As far as your two questions go:
> I have also merged the disk API versions because the zones parameter was only supported in newer API versions. I do not fully understand the impact of bumping the API versions, but if tests pass, I think it should be fine. Thoughts?
Integration tests would increase the confidence here, but sadly we don't have integration tests for Azure ARM.
> The new API version also caused request and response payloads to differ i.e. the volume types are now passed as [DiskSku](https://docs.microsoft.com/en-us/rest/api/compute/disks/create-or-update?tabs=HTTP#disksku) objects instead of strings. How do we handle API changes? Is there documentation around making such API changes?
One option would be to allow user to pass ``api_version`` or similar argument to the constructor and then when user specifies a new version which uses new ``DISK_API_VERSION`` value, otherwise it would default to the previous 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] Kami merged pull request #1736: azure-arm: Add support to create disks in specific availability zones
Posted by GitBox <gi...@apache.org>.
Kami merged PR #1736:
URL: https://github.com/apache/libcloud/pull/1736
--
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