Fix Physical Disks save inventory #17972
Conversation
EsdrasVP
changed the title
EsdrasVP
force-pushed
the
fix_disks_save_inventory
branch
from
7f140ea to
df8626c
Compare
EsdrasVP
force-pushed
the
fix_disks_save_inventory
branch
from
df8626c to
13d88e1
Compare
EsdrasVP
changed the title
|
Request related to RFE: https://bugzilla.redhat.com/show_bug.cgi?id=1629900 |
| save_inventory_single(:physical_disk, parent, hash) | ||
| def save_physical_disks_inventory(physical_storage, hashes, target = nil) | ||
| return if hashes.nil? | ||
| deletes = target == physical_storage ? :use_association : [] |
There was a problem hiding this comment.
For these child collections you should just use the association as long as the target type cannot an instance of this collection.
Unless you expect to actually get physical_storage as a top-level refresh target just pass :use_association into save_inventory_multi
There was a problem hiding this comment.
The end result of this will be that no physical_disks will ever be deleted, because the target will always be the EMS.
There was a problem hiding this comment.
For these child collections you should just use the association as long as the target type cannot an instance of this collection.
Unless you expect to actually get physical_storage as a top-level refresh target just pass:use_associationintosave_inventory_multi
@agrare I changed it to use :use_association unless PhysicalStorage is assigned as the target since these disks are related to physical storage units, this also solves the deletion problem. Though I'm still thinking if it's interesting to use EMS instead of PhysicalStorage as default in case other providers decide to use this model. What do you think?
EsdrasVP
force-pushed
the
fix_disks_save_inventory
branch
from
13d88e1 to
3251e00
Compare
| def save_physical_disks_inventory(physical_storage, hashes, target = nil) | ||
| return if hashes.nil? | ||
| target = physical_storage if target.nil? | ||
| deletes = target == physical_storage ? :use_association : [] |
There was a problem hiding this comment.
Actually since these are only under a physical storage, there is no case where you wouldn't want to :use_association
This is usually meant for top level collections (e.g. if you had ems.physical_disks and you target e.g. a physical_host not the full ems).
Just take this out and pass :use_association to save_inventory_multi
There was a problem hiding this comment.
@agrare Ok, got it, I changed it to pass :use_association directly to save_inventory_multi.
EsdrasVP
force-pushed
the
fix_disks_save_inventory
branch
from
3251e00 to
2f83774
Compare
|
Checked commit EsdrasVP@2f83774 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
This PR is able to: