libcloud
libcloud copied to clipboard
chore: Remove storage and volume interface implementation
Changes Title (replace this with a logical title for your changes)
Remove unused and unsupported apis from equinix metal
Description
- Remove implementation of storage APIs as they were removed from Packet years ago. there is no direct replacement. Equinix Metal offers Pure and NetApp storage devices today but there is not equivalent functionality as far as the Equinix Metal API is concerned with directly attaching storage to servers and equinix metal doesnot supports only 2 storage providers and
For more information on contributing, please see Contributing section of our documentation.
Status
Ready For Review
Checklist (tick everything that applies)
- [x] Code linting (required, can be done after the PR checks)
- [x] Documentation
- [x] Tests
- [x] ICLA (required for bigger changes)
Reviewer note: The Elastic Block storage feature was removed from Equinix Metal in June 2021. https://web.archive.org/web/20210620005004/https://metal.equinix.com/developers/docs/storage/elastic-block-storage/#elastic-block-storage
@Kami is this something that you can review and merge? There are three Equinix Metal PRs up. I have reviewed them on their Equinix Metal merits.
Thanks for the contribution.
Since this is a breaking change, can you please add an entry in upgrade notes file (docs/upgrade_notes.rst)? Thanks.
Codecov Report
Merging #1972 (86ff77e) into trunk (dfbb595) will increase coverage by
0.06%. Report is 10 commits behind head on trunk. The diff coverage isn/a.
Additional details and impacted files
@@ Coverage Diff @@
## trunk #1972 +/- ##
==========================================
+ Coverage 83.20% 83.25% +0.06%
==========================================
Files 353 353
Lines 81423 81252 -171
Branches 8591 8559 -32
==========================================
- Hits 67742 67645 -97
+ Misses 10874 10820 -54
+ Partials 2807 2787 -20
| Files | Coverage Δ | |
|---|---|---|
| libcloud/compute/drivers/equinixmetal.py | 70.54% <ø> (+6.18%) |
:arrow_up: |
| libcloud/test/compute/test_equinixmetal.py | 92.89% <ø> (+2.76%) |
:arrow_up: |
@aayushrangwala Can you please sync up this branch with a latest trunk? I wanted to wrap it up myself locally, but I noticed more work is needed to make this change complete - it appears create_node() still calls create_volume() / attach_volume(), etc.
Please let me know when that has been addressed and when all checks are passing and I will have a look again.
On that note, it would also be good to update PR description with some context and why this change is needed (I know that context may be available in other PR, but we should also update this PR description).
Thanks.
@Kami can you please review again, Ive rebased and updated.
@antoinebourayne I had a look and it looks like this comment is still relevant - https://github.com/apache/libcloud/pull/1972#issuecomment-1837447866
I see create_node() method is still calling volume related methods which have been removed. create_node() should be updated to remove those calls and disk + disk_size argument should be removed as well (since it doesn't make sense / it's unused without corresponding volume management methods).
I made and pushed the following changes myself:
- Remove disk and disk_size argument from create_node method - daddbae3fe699dcc708e2f7585a5d6865875ce7a
- Add upgrade notes entry - 70a5f872c97d771dab2c5fe4e66a9b8a1d3aaf8a
- Remove
volumesfromlist_resources_async()- fc48cacde37b21498e4c7f1d597392df238c3c3f - Fix lint - 84b68be73045532110aa92eaa0b05873663ed994
With those changes, the code has now also been merged into trunk.