Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: Deprecate facility in the favor of metro #1971

Merged
merged 5 commits into from
Apr 18, 2024
Merged

chore: Deprecate facility in the favor of metro #1971

merged 5 commits into from
Apr 18, 2024

Conversation

aayushrangwala
Copy link
Contributor

@aayushrangwala aayushrangwala commented Nov 9, 2023 •
edited
Loading

Changes Title (replace this with a logical title for your changes)

Deprecate facility in the favor of metro API

Description

  • Support Metros location in libcloud interfaces and deprecate facility api.
  • Also, 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)

  • Code linting (required, can be done after the PR checks)
  • Documentation
  • Tests
  • ICLA (required for bigger changes)

displague
displague reviewed Nov 9, 2023
View reviewed changes
libcloud/compute/drivers/equinixmetal.py Outdated Show resolved Hide resolved
libcloud/compute/drivers/equinixmetal.py Outdated Show resolved Hide resolved
@displague
Copy link

@aayushrangwala perhaps the removal of strorage should be done in a separate PR from the facility deprecation and introduction of metros.

@aayushrangwala aayushrangwala force-pushed the equinix-metal-deprecate-facility branch from 1d3e36f to 3f5b662 Compare November 10, 2023 13:48
@aayushrangwala aayushrangwala marked this pull request as ready for review November 10, 2023 13:49
@aayushrangwala aayushrangwala changed the title Deprecate facility in the favor of metro chore: Deprecate facility in the favor of metro Nov 10, 2023
@luss
Copy link
Contributor

luss commented Nov 10, 2023

nice i"ll puul this PR into my work area and make sure PR #1970 & PR #1971 are all thats needed to tqeak EqunixMetal back into prime shape.

@luss
Copy link
Contributor

luss commented Nov 10, 2023

Also, I agree with @displague that it's cleaner, safer, & simpler to keep each PR small & doing a specifc thing. That's why I started this journey to update the EquinixMetal driver with the simple and safe PR #1970

@aayushrangwala
Copy link
Contributor Author

@aayushrangwala perhaps the removal of strorage should be done in a separate PR from the facility deprecation and introduction of metros.

Sure, will raise another PR

@aayushrangwala aayushrangwala force-pushed the equinix-metal-deprecate-facility branch 2 times, most recently from ae20446 to ea215b8 Compare November 12, 2023 09:37
@displague
Copy link

displague commented Nov 13, 2023 •
edited
Loading

Reviewers, please note that Equinix Metal removed the facilities/facility request field in June for new users, and the field will be entirely unavailable for all users after December. metro is the replacement. Facilities reside within a metro. Legacy facility values are not compatible with the metro field.

https://feedback.equinixmetal.com/changelog/reminder-facilities-turns-to-metros-on-may-30

@displague displague mentioned this pull request Nov 13, 2023
Closed
4 tasks
@aayushrangwala
Copy link
Contributor Author

@Kami Can you please review and approve this PR as well

Kami
Kami approved these changes Dec 3, 2023
View reviewed changes
Copy link
Member

@Kami Kami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the contribution.

@Kami
Copy link
Member

Kami commented Dec 3, 2023

@aayushrangwala Looks like tests and some other CI checks are failing. Can you please look into it when you get a chance?

Thanks.

@Kami Kami added this to the v3.9.0 milestone Dec 3, 2023
@aayushrangwala
Deprecate facility in the favor of metro
fcd0b43 
    Signed-off-by: Ayush Rangwala <ayush.rangwala@gmail.com>
@aayushrangwala
fix: lint issues
adf831d 
Signed-off-by: Ayush Rangwala <ayush.rangwala@gmail.com>
@aayushrangwala aayushrangwala force-pushed the equinix-metal-deprecate-facility branch from 5f6a837 to adf831d Compare January 17, 2024 17:08
displague
displague suggested changes Jan 19, 2024
View reviewed changes
libcloud/compute/drivers/equinixmetal.py Outdated
@@ -252,8 +252,12 @@ def ex_list_nodes_for_project(self, ex_project_id, include="plan", page=1, per_p
return list(map(self._to_node, data))

def list_locations(self):
<<<<<<< HEAD
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad merge - several unresolved conflicts in this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes missed that file to be committed, resolved

@aayushrangwala
Fix: merge conflicts
43ca4b4 
Signed-off-by: Ayush Rangwala <ayush.rangwala@gmail.com>
@aayushrangwala
Add back spaces
92a86c2 
Signed-off-by: Ayush Rangwala <ayush.rangwala@gmail.com>
displague
displague approved these changes Jan 21, 2024
View reviewed changes
Copy link

@displague displague left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

asfgit pushed a commit that referenced this pull request Apr 18, 2024
@Kami
Add changelog entry for #1971.
74a9a21
@asfgit asfgit merged commit 5fcbcd6 into apache:trunk Apr 18, 2024
15 of 17 checks passed
@Kami
Copy link
Member

Kami commented Apr 18, 2024

Sorry for the delay - the PR has finally been merged into trunk.

Thanks again for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@displague displague displague approved these changes

@Kami Kami Kami approved these changes

Assignees
No one assigned
Labels
api: compute drivers: equinix metal
Projects
None yet
Milestone
v3.9.0
Development

Successfully merging this pull request may close these issues.
5 participants
@aayushrangwala @displague @luss @Kami @asfgit