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/05/25 15:08:59 UTC
[GitHub] [libcloud] Eis-D-Z opened a new pull request, #1699: Get GCE image price
Eis-D-Z opened a new pull request, #1699:
URL: https://github.com/apache/libcloud/pull/1699
## Add new method to get gce image price
Add tests
### Description
Getting GCE image price right now with
`libcloud.pricing.get_pricing(driver_type='compute', driver_name='gce_images')`
is broken. I added a separate method to do that since gce images price dictionary veers off the pattern of the rest of the pricing data. I added some tests that use directly the pricing data and not the mock pricing data, on purpose, the main reason is to detect any changes in the format of the pricing data by Google.
### 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)
- [x] Documentation
- [x] [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] Kami commented on a diff in pull request #1699: Get GCE image price
Posted by GitBox <gi...@apache.org>.
Kami commented on code in PR #1699:
URL: https://github.com/apache/libcloud/pull/1699#discussion_r889676868
##########
libcloud/pricing.py:
##########
@@ -200,6 +202,71 @@ def get_size_price(driver_type, driver_name, size_id, region=None):
return price
+def get_gce_image_price(image_name, size_name, cores=1):
Review Comment:
Would it be possible to make this "private" and call it inside ``get_size_price()`` method when GCE pricing is requested?
Since the whole idea behind Libcloud is to try to use common / standard API as much as possible. If we create a provider specific methods everywhere it kinda defeats the purpose (yes, I know it's not always possible to do it in common / standard way, but we should try out best).
--
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 #1699: Get GCE image price
Posted by GitBox <gi...@apache.org>.
Kami commented on PR #1699:
URL: https://github.com/apache/libcloud/pull/1699#issuecomment-1193159499
@Eis-D-Z Changes look good to me, but unit tests appear to be failing. Can you please have a look when you get a chance? 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] Eis-D-Z commented on pull request #1699: Get GCE image price
Posted by GitBox <gi...@apache.org>.
Eis-D-Z commented on PR #1699:
URL: https://github.com/apache/libcloud/pull/1699#issuecomment-1147939458
Hey Tomaz,
Of course, sorry for the delay. I am on it.
--
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] Eis-D-Z commented on pull request #1699: Get GCE image price
Posted by GitBox <gi...@apache.org>.
Eis-D-Z commented on PR #1699:
URL: https://github.com/apache/libcloud/pull/1699#issuecomment-1237324156
@Kami Excuse me for the delay.
--
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 #1699: Get GCE image price
Posted by GitBox <gi...@apache.org>.
Kami commented on PR #1699:
URL: https://github.com/apache/libcloud/pull/1699#issuecomment-1146780196
Short therm this change looks good to me, but long term we should figure out how we can integrate it into standard ``get_size_pricing()`` method.
--
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 #1699: Get GCE image price
Posted by GitBox <gi...@apache.org>.
Kami commented on PR #1699:
URL: https://github.com/apache/libcloud/pull/1699#issuecomment-1146780464
@Eis-D-Z Thanks for the contribution. Would it be possible to implement something like this https://github.com/apache/libcloud/pull/1699/files#r889676868?
--
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 #1699: Get GCE image price
Posted by GitBox <gi...@apache.org>.
Kami merged PR #1699:
URL: https://github.com/apache/libcloud/pull/1699
--
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 #1699: Get GCE image price
Posted by GitBox <gi...@apache.org>.
Kami commented on PR #1699:
URL: https://github.com/apache/libcloud/pull/1699#issuecomment-1236669381
@Eis-D-Z How is this PR looking? Would be great if we could get it wrapped up merged. 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 #1699: Get GCE image price
Posted by GitBox <gi...@apache.org>.
Kami commented on PR #1699:
URL: https://github.com/apache/libcloud/pull/1699#issuecomment-1169176316
@Eis-D-Z
1) In this case, to reduce confusion it would probably be better to introduce a new function, e.g. ``get_image_price`` or similar.
2) If we go with 1), I assume those arguments will then be added to this new function? I guess it may still be somewhat confusing since "cores" implies size to some extent, but I know that some image licensing is based on number of cores, so it probably also fine / make sense to add it there.
--
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] Eis-D-Z commented on pull request #1699: Get GCE image price
Posted by GitBox <gi...@apache.org>.
Eis-D-Z commented on PR #1699:
URL: https://github.com/apache/libcloud/pull/1699#issuecomment-1158694103
Hi Tomaz,
I'm very sorry, I got some issues and only just now resolved them. I can
continue to improve the PR to be inline with the overall philosophy.
I do have some questions though:
1) get_size_price has `size` in its name so it might be misleading to
return image price, do you want to rename the method or are you cool with
this.
2) I will need to add two arguments, `image_name` and `cores`, and naming
is my weak point. Are those names alright?
3) I am preparing another PR with prices for EC2, because AWS have added to
their official endpoint prices. (The same endpoint where we get the sizes)
I will open it within the day on a separate PR, I believe you wouldn't want
to add it in this PR.
Cheers
Alex
--
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 #1699: Get GCE image price
Posted by GitBox <gi...@apache.org>.
Kami commented on code in PR #1699:
URL: https://github.com/apache/libcloud/pull/1699#discussion_r889676868
##########
libcloud/pricing.py:
##########
@@ -200,6 +202,71 @@ def get_size_price(driver_type, driver_name, size_id, region=None):
return price
+def get_gce_image_price(image_name, size_name, cores=1):
Review Comment:
Would it be possible to make this "private" and call it inside ``get_size_price()`` method when GCE pricing is requested?
Since the whole idea behind Libcloud is to try to use common / standard API as much as possible. If we create a provider specific methods everywhere it kinda defeats the purpose (yes, I know it's not always possible to do it in common / standard way, but we should try our best).
--
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 #1699: Get GCE image price
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1699:
URL: https://github.com/apache/libcloud/pull/1699#issuecomment-1137424504
# [Codecov](https://codecov.io/gh/apache/libcloud/pull/1699?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 [#1699](https://codecov.io/gh/apache/libcloud/pull/1699?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (36ff4c3) into [trunk](https://codecov.io/gh/apache/libcloud/commit/83d1e86aff1109051c417714ea5986eadf3bbf5f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (83d1e86) will **increase** coverage by `0.00%`.
> The diff coverage is `83.33%`.
[](https://codecov.io/gh/apache/libcloud/pull/1699?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 #1699 +/- ##
=======================================
Coverage 83.29% 83.29%
=======================================
Files 400 400
Lines 87755 87839 +84
Branches 9328 9345 +17
=======================================
+ Hits 73092 73162 +70
- Misses 11485 11493 +8
- Partials 3178 3184 +6
```
| [Impacted Files](https://codecov.io/gh/apache/libcloud/pull/1699?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/pricing.py](https://codecov.io/gh/apache/libcloud/pull/1699/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-bGliY2xvdWQvcHJpY2luZy5weQ==) | `69.53% <70.21%> (+0.39%)` | :arrow_up: |
| [libcloud/test/test\_pricing.py](https://codecov.io/gh/apache/libcloud/pull/1699/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-bGliY2xvdWQvdGVzdC90ZXN0X3ByaWNpbmcucHk=) | `97.56% <100.00%> (+1.04%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/libcloud/pull/1699?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/1699?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 [83d1e86...36ff4c3](https://codecov.io/gh/apache/libcloud/pull/1699?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] Eis-D-Z commented on pull request #1699: Get GCE image price
Posted by GitBox <gi...@apache.org>.
Eis-D-Z commented on PR #1699:
URL: https://github.com/apache/libcloud/pull/1699#issuecomment-1173543645
Ok I will go with 1.
Message ID: ***@***.***>
--
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