Mailinglist Archive: yast-devel (87 mails)

< Previous Next >
Re: [yast-devel] Feedback needed! Hack Week project to re-define storage-ng API
On 02/24/2017 10:37 AM, Josef Reidinger wrote:
On Thu, 23 Feb 2017 17:45:48 +0100
Ancor Gonzalez Sosa <ancor@xxxxxxx> wrote:

My Hack Week project is in that state in which all the important parts
work and now it's time to decide if it's worth investing a couple of
extra days to finish it all the way down.

Since the ultimate goal is to change how YaST interacts with
storage-ng, I would need the opinion of as many YaST developers as
possible.

So please, please, please, take a look to this and give your opinion.
https://github.com/yast/yast-storage-ng/pull/169

The most important question is - do you prefer the proposed approach
and API* or do you prefer to continue accessing directly to
libstorage-ng from YaST?

Thanks

* In general, minor details in the API can be ironed out in the short
future.

let me comment it point by point:

Arrays instead of strictly-typed vectors
----------------------------------------

I think it make sense to have a bit specialized arrays, but it do not
make sense to not act as array, so for me the best way would be to have
something like `class MountPoints < Array` which have special filters
like mountpoints.snapshots which return again MountPoints, but only
btrfs snapshots. This allow easy extending in future and still it act
like array.

We have DevicesList and all its subclasses for that (already presented
in a mail some time ago, usually referred as "query API" and extensively
used in yast2-bootloader, for example). But since that was already
provided by the current yast2-storage-ng, I wanted to leave it out of
the discussion. With this new API, DevicesList & friends would still be
there but MUCH, MUCH better integrated (no more tricks or refinements).

For example, you could do things like
dvcegraph.all_filesystems.with_mountpoint("/boot")

disk.all_partitions.with {|p| p.size > 10.GiB && p.boot? }.map(&:name)

dvcegraph.all_disks.with(name: /vd/).all_partitions.with(number: 1)


No more downcasting
-------------------

good improvement


Nil instead of exceptions
-------------------------

make sense for me, another option that make sense for me is NullObject,
that allows some introspection and e.g. better writting to log.

Classes instead of constants and enums
-------------------------------------

nice improvement

All sizes as DiskSize objects
-----------------------------

makes sense as it is not only integer, but have its own logic


Adding methods needed by YaST
-----------------------------

makes sense


Less surprising API (for Rubyists)
----------------------------------

nice just one more idea. I really hate to type everywhere staging
graph. So why not have `def all(graph = Storage.staging)` as I think
majority of other Yast modules are interested mainly in staging graph.

Well, being picky it would be
def all(graph = StorageManager.instance.staging)
but ok. :-)

Overall I like it.

Josef

Cheers.
--
Ancor González Sosa
YaST Team at SUSE Linux GmbH
--
To unsubscribe, e-mail: yast-devel+unsubscribe@xxxxxxxxxxxx
To contact the owner, e-mail: yast-devel+owner@xxxxxxxxxxxx

< Previous Next >