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.

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

Complete Transition from AvailabilityMixin to SupportsFeatureMixin #21179

Closed
37 tasks done
chessbyte opened this issue Apr 16, 2021 · 12 comments
Closed
37 tasks done

Complete Transition from AvailabilityMixin to SupportsFeatureMixin #21179

chessbyte opened this issue Apr 16, 2021 · 12 comments
Assignees
Projects
Milestone

Comments

@chessbyte
Copy link
Member

chessbyte commented Apr 16, 2021

@chessbyte
Copy link
Member Author

@agrare is this validate_reboot valid? Is it for reboot_guest, reset or another operation?

@agrare
Copy link
Member

agrare commented Jun 14, 2021

@chessbyte looks like validate_reboot is only used by Host, and that instance in the powervs vm class is not valid since it already has supports for reboot_guest and reset. I'll put in a PR to remove it. nevermind you already did that

@kbrock
Copy link
Member

kbrock commented Nov 2, 2021

How do we know when we are done with this?

I do see a few calls to is_valid?, but we also have a few bridge methods that will mask the usage.

Is there a particular pattern we can look for in the contents of def validate_* for us to know if it is a validation or part of this plugin? (or are these defined in a meta way so we can just focus on

@chessbyte
Copy link
Member Author

See OP regarding AvailabilityMixin, is_available? and is_available_now_error_message

@Fryguy
Copy link
Member

Fryguy commented May 25, 2022

@kbrock is this done?

@kbrock
Copy link
Member

kbrock commented May 31, 2022

not done, I will add a few more notes

@chessbyte
Copy link
Member Author

@chessbyte
Copy link
Member Author

@Fryguy @kbrock either this should be reopened to get the above to a zero count or moved to Oparin

@agrare
Copy link
Member

agrare commented Aug 15, 2022

We should definitely get those counts to 0, but it looks like none of these are "live" code they are either comments, spec test context names that are now testing supports?, or methods named is_available that are not from the AvailabilityMixin (e.g. MiqIPMI.is_available? is not from AvailabilityMixin)

@kbrock do you want to delete these comments and rename the spec contexts?

@Fryguy
Copy link
Member

Fryguy commented Aug 15, 2022

At a minimum we shouldn't have a closed issue that is "In Progess" on the Roadmap - @kbrock Let me know which direction I should go.

@kbrock
Copy link
Member

kbrock commented Aug 17, 2022

searches of interest:

Most of the references were comments and MiqIPMI that @agrare mentioned.
I was able to fine two reference that slipped through the cracks.

One of those 2 references is a shim that allows for backwards compatibility. So nothing is broken. I agree, we should have removed both of these before we marked this as done.

comments:

@Fryguy
Copy link
Member

Fryguy commented Aug 18, 2022

#21989 includes the rest of the followups

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Roadmap
  
Oparin
Development

No branches or pull requests

4 participants