Mailinglist Archive: yast-devel (211 mails)

< Previous Next >
Re: [yast-devel] Some notes about new storage API
On Tue, Oct 13, 2015 at 10:14:03AM +0200, Josef Reidinger wrote:

I'll do more experiments with this as soon as the prototype is easily
installable. But looks pretty clear that it would be fairly easy to
fix the un-rubism by adding the corresponding methods in the Ruby
side in case we decide we are so purist that we cannot live with the
current API. :-) So nothing to fix in the libstorage (C++) side.

I agree, that it make sense to have some helpers to have ruby
bindings more "ruby".

Just remember that those need documentation and test cases.

a) Is device_graph always an object representing the whole
graph?

The devicegraph always represents the whole graph.


Well, in general I do not like this API before and I also do not like
it now :)
It is not object oriented and whats more device_graph is god like
object.

Please elaborate this. What do you mean by God like object? A
object that holds all storage objects?

What I expect is object like API that looks like

device_graph.find_device(id)

This exists.

device_graph.find_device("/dev/sda", block: true)

Not type safe.

this way it is more separated responsibility. In general now
device_graph looks like target map, which I would like to
avoid.

Please explain that, esp. what problems you see with the existing
target map and how you still see this problems with the new
design.

I prefer to have graph of objects that points to each other

The device-graph is just that and the functions to query the
"pointers" exist.

and it is easy to say what object it returns. It also allows
easy introspection and better documentation that allows easier
usability.

In general your remarks are to buzz-work like to comment on them.

In theory there are many ways to get a subgraph, look at the
functions in Device.h.

Storage::Filesystem.find_by_mountpoint(sub_graph, "/")

Do you have a real use-case for that? Adding subgraphs looks like
a lot of work so might not be worthwhile.

No concrete example, just wondering. But the idea of restricting the
search to a subtree somehow came naturally to my mind while reading.
Like "look for a filesystem with this property, but only in this
volume group".

Probably not worth the effort, I was just curious.

For bootloader it make sense to e.g. restrict search for boot flag for
one disk, as it is disk specific flag.

Then I will look at the subgraph and filtered_graph classes of
boost.

Same way it can be interesting
to e.g. find intersections for known problem cases like
`device_graph.find(fs:
"nfs).include?(device_graph.mount_points.get("/boot")`

That's a bad example FMPOV, instead just search for /boot and
check whether it is NFS. No need to construct two filtered graphs
and check if they intersect.


b) Adding find_by_xx at demand over time doesn't look very
future-proof. I would prefer to see something like Rails's
#find_by, i.e.

Storage::Filesystem.find(device_graph, mountpoint: "/")
Storage::Filesystem.find(device_graph, mountpoint: "/", label:
"blah")

Too ruby-centric and not easy to implement in a cross-language
way, I guess. But asking does not hurt. :-)

As I already wrote in
http://lists.opensuse.org/yast-devel/2014-12/msg00028.html if
someone takes care of bindings for a target language more things
are possible.

Yes. And it should be fairly easy to implement. But I still wonder
about how flexible and future-proof is to keep adding find_by_xx on
demand to the API.

I agree, almost all languages have maps that can be used to pass such
kind of options. In C it is usually done by flags. In C++ it is usually
done by functor. So maybe another option is to have generic find which
can take functor or lambda in way like

device_graph.mountpoints.find { |mp| mp.path == "/" || mp.label ==
"blah" }

A filtered graph constructed by a functor should be possible.

More flexible solutions would also be more complex to use for sure. I
just wonder if they are worth exploring. One obvious solution (that
would need refinement, of course) if having a search object that
accepts any number of key-pair filters. The most primitive form just
to expose the idea would be something like (in pseudocode):

s = Storage.search(device_graph, "filesystem")
s.add_filter("mountpoint", "/")
s.first()

Or any other idea that can be implemented with a fixed set of classes
and methods instead of extending the existing classes with new custom
methods over time.

Agreed. number for methods for given object should be kept quite low,
otherwise it is hard to remember it and also learn it.

In general agreed, but if you have a get_xx and a set_xx function
a find_by_xx seems natural.

I think that code is the best documentation if API is small and have
good names.

:)

ciao Arvin

--
Arvin Schnell, <aschnell@xxxxxxxx>
Senior Software Engineer, Research & Development
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284
(AG Nürnberg)
Maxfeldstraße 5
90409 Nürnberg
Germany
--
To unsubscribe, e-mail: yast-devel+unsubscribe@xxxxxxxxxxxx
To contact the owner, e-mail: yast-devel+owner@xxxxxxxxxxxx

< Previous Next >
Follow Ups