Move supports :create to Openstack provider repo #613
Conversation
agrare
force-pushed
the
move_supports_create_flavor
branch
from
c3f271f to
c4fe7e9
Compare
| @@ -1,6 +1,11 @@ | |||
| class ManageIQ::Providers::Openstack::CloudManager::Flavor < ::Flavor | |||
| include ManageIQ::Providers::Openstack::HelperMethods | |||
|
|
|||
| supports :create | |||
| supports :delete do | |||
| unsupported_reason_add(:delete, _("The Flavor is not connected to an active Provider")) if ext_management_system.nil? | |||
There was a problem hiding this comment.
shouldn't it be checking whether ext_management_system.nil? on :create as well? in fact, the original code in this file seems to have been checking only on :create and not on :delete
There was a problem hiding this comment.
shouldn't it be checking whether ext_management_system.nil? on :create as well
There is no flavor instance to check on create since we're trying to create it. The supports feature blocks are only invoked on the instance level not the class level.
in fact, the original code in this file seems to have been checking only on :create and not on :delete
Yeah I don't think those validations make any sense (see ManageIQ/manageiq#20332 (comment)). We're validating that when we create a flavor that it has an EMS but it isn't created yet.
There was a problem hiding this comment.
Going to simplify this for this bugfix and leave the validates_* methods and just move the supports * calls
Currently the supports :create/:delete features are enabled at the base Flavor model, however only Openstack implements these operations
agrare
force-pushed
the
move_supports_create_flavor
branch
from
c4fe7e9 to
cc98f90
Compare
|
Checked commit agrare@cc98f90 with ruby 2.5.7, rubocop 0.69.0, haml-lint 0.28.0, and yamllint |
Move supports :create to Openstack provider repo (cherry picked from commit 78cdb0b)
|
Jansa backport details: |
Currently the supports :create/:delete features are enabled at the base Flavor model, however only Openstack implements these operations
Cross Repo Tests: ManageIQ/manageiq-cross_repo-tests#149
Dependent PR: ManageIQ/manageiq#20332