Subclass IsoDatastore under Storage #17
Conversation
nasark
force-pushed
the
move_iso_datastore
branch
from
6cab720 to
4b52799
Compare
|
Some comments on commit nasark@4b52799 spec/models/manageiq/providers/redhat/infra_manager/iso_datastore_spec.rb
|
|
Checked commit nasark@4b52799 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint spec/models/manageiq/providers/redhat/infra_manager/ovirt_services/v4_spec.rb
|
|
|
||
| iso_images |
There was a problem hiding this comment.
Since OvirtServices is a simple class we should be able to simply inherit from Ovirt::InfraManager::OvirtServices without duplicating the code. Don't want to hold this up but we can probably delete all this code in a follow-up
There was a problem hiding this comment.
Thanks, I'll take note for a future follow-up PR
| def storagedomains | ||
| collector.storagedomains.each do |storagedomain| | ||
| storage_type = storagedomain.dig(:storage, :type).upcase | ||
| ems_ref = ManageIQ::Providers::Redhat::InfraManager.make_ems_ref(storagedomain.try(:href)) | ||
| location = case storage_type | ||
| when 'LOCALFS', 'ISO' | ||
| ems_ref | ||
| when 'NFS', 'GLUSTERFS' | ||
| "#{storagedomain.dig(:storage, :address)}:#{storagedomain.dig(:storage, :path)}" | ||
| else | ||
| storagedomain.dig(:storage, :volume_group, :id) | ||
| end | ||
|
|
||
| free = storagedomain.try(:available).to_i | ||
| used = storagedomain.try(:used).to_i | ||
| total = free + used | ||
| committed = storagedomain.try(:committed).to_i | ||
|
|
||
| storage_domain_type = storagedomain.dig(:type, :downcase) | ||
| type = storage_domain_type == 'iso' ? "IsoDatastore" : "Storage" | ||
|
|
||
| persister.storages.find_or_build(ems_ref).assign_attributes( | ||
| :ems_ref => ems_ref, | ||
| :name => storagedomain.try(:name), | ||
| :store_type => storage_type, | ||
| :storage_domain_type => storage_domain_type, | ||
| :total_space => total, | ||
| :free_space => free, | ||
| :uncommitted => total - committed, | ||
| :multiplehostaccess => true, | ||
| :location => location, | ||
| :master => storagedomain.try(:master), | ||
| :type => "ManageIQ::Providers::Redhat::InfraManager::#{type}" | ||
| ) | ||
| end | ||
| end |
There was a problem hiding this comment.
Similar comment here, if we let auto_sti_class work then we don't need to specify :type => "ManageIQ::Providers::Redhat::InfraManager::#{type}" then we don't need to override this method at all.
@miq-bot add_label enhancement
@miq-bot assign @agrare