On 03/30/2017 06:53 PM, Ancor Gonzalez Sosa wrote:
> This is a follow-up on this discussion
> https://github.com/yast/yast-storage-ng/pull/184#pullrequestreview-29672717
>
> The mail is mainly targeted to Josef, so we continue the discussion in a
> more persistent and prominent place than a merged pull request. Don't
> feel guilty if you decide to ignore it. :-)
>
> Before introducing the Y2Storage wrapper classes on top of the Storage
> ones, we had a query API (already discussed in this list) based on the
> concept of Y2Storage::DevicesLists.
>
> I think we don't need that anymore with the wrapper. I would go for an
> approach based on pure Ruby arrays, now that we don't have to fear
> not-found exceptions or to care about downcasting.
So I decided to do another test to validate that idea. Let's take
existing code using the DeviceLists superpowers and compare how it would
look like with plain arrays of wrapped objects.
1)
# current
efi_partition =
devgraph.blk_filesystems.with_mountpoint("/boot/efi").partitions.first
# wrapper + plain arrays
efi_partition =
devgraph.partitions.detect do |p|
p.blk_filesystem && p.blk_filesystem.mountpoint == "/boot/efi"
end
2)
# current
mbr_disk = devgraph.disks.with(name: mbr_disk_name).first
# wrapper + plain arrays
mbr_disk = devgraph.disks.detect {|d| d.name == "mbr_disk_name" }
3)
# current
partitions =
devgraph.disks.with(name: disk.name).partitions.reject do |part|
[Storage::ID_SWAP, Storage::ID_GPT_BIOS].include?(part.id)
end
# wrapper + plain arrays
partitions =
devgraph.partitions.select |part|
part.disk && part.disk.name == disk.name &&
![:swap, :gpt_bios].include?(part.id.to_sym)
end
4)
# current
def part_or_disk_at_mountpoint(mntpnt)
fses = staging.filesystems.with_mountpoint(mntpnt)
return nil if fses.empty?
partitions = fses.partitions
partitions = fses.lvm_vgs.partitions if partitions.empty?
return partitions.first unless partitions.empty?
disks = fses.disks
disks = fses.lvm_vgs.disks if disks.empty?
disks.first
end
# wrapper + plain arrays
def part_or_disk_at_mountpoint(mntpnt)
fs = staging.filesystems.detect {|f| f.mountpoint == "mntpnt"}
return nil unless fs
fs.ancestors.detect {|a| a.partition? } ||
fs.ancestors.detect {|a| a.disk? }
end
Those four examples should be enough as an exercise. In general it looks
like more code because we don't have the #with shortcut (it was useful
as a shortcut, but its real motivation was to deal with Storage
un-rubistm), but I still think it's ok since now is straightforward
common Ruby code with no magic.
--
Ancor González Sosa
YaST Team at SUSE Linux GmbH
--
To unsubscribe, e-mail: [email protected]
To contact the owner, e-mail: [email protected]
