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, <[email protected]>
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: [email protected]
To contact the owner, e-mail: [email protected]
