I have a design decision to take and I need advise. Please, stay on topic (i.e. "design a ruby layer to query the not-under-discussion structure of devicegraphs coming from libstorage").
While writing the code for the new proposal we needed to query a devicegraph (hopefully you all know what a devicegraph is in the libstorage-ng world) for things like:
- all free spaces in a given subset of disks (candidates for install) - all filesystems in the same subset, not caring if they are directly on the disk or on a partition table
While writing tests, we found we had similar requirements but without constraining those queries to a set of disks (i.e. we wanted to list the free spaces, partitions, filesystems, etc. at some point of the whole devicegraph).
We came up with the idea of a Devicegraph query object that is already partially implemented and seems to solve our problems. But if we are going to start using it in many places (there will be calls in almost every single test) I want to make sure the API does not suck much. I have two alternatives and would like to hear/read opinions.
As usual, we don't want to over-engineer it, but we don't want to throw it away with the next requirement change either.
Explanation by example, hopefully enough (ping me if it's not). Please read both options before commenting (I actually like more the second one).
Option 1 - Only a DevicegraphQuery class
https://gist.github.com/ancorgs/9c628ef0f4fa717a2817
But, what about something like this? more_fine_query.disks
With the implementation I have in mind, it would return the same that fine_query.disks, but some people could expect it to return only disks from fine_query.disks which, as additional requirement, contain any primary partition.
This, of course, can be solved documenting which restrictions are honored by each query method (for example, #disks would only honor #with_disk while #partitions would honor both #with_disk and #with_partition).
That is, the expected direction of the hierarchy (like "a filesystem can be expected at some point below a disk, but not the other way around") would be in the source code and documentation of the methods at DevicegraphQuery.
Option 2 - A WhateverQuery class per type of object
https://gist.github.com/ancorgs/a98597fa9a5e348eaaac
This is my preferred option for several reasons, even if it means implementing more classes (something that is not bad, per se).
First of all, the direction of the hierarchy is reflected in the API (DiskQuery has a method #filesystems which returns a FilesystemQuery but not the other way around).
It also makes possible (and easy) to write specific filters for a given type of query, like # Get an array of free spaces big enough to be used for something FreeDiskSpaceQuery.new(devicegraph).useful.to_a
I also find it more readable for the tests and very Ruby-like (even Rails-like), but that can be a matter of taste.
Opinions?
On Fri, 18 Mar 2016 09:57:11 +0100 Ancor Gonzalez Sosa ancor@suse.de wrote:
I have a design decision to take and I need advise. Please, stay on topic (i.e. "design a ruby layer to query the not-under-discussion structure of devicegraphs coming from libstorage").
While writing the code for the new proposal we needed to query a devicegraph (hopefully you all know what a devicegraph is in the libstorage-ng world) for things like:
- all free spaces in a given subset of disks (candidates for install)
- all filesystems in the same subset, not caring if they are directly
on the disk or on a partition table
While writing tests, we found we had similar requirements but without constraining those queries to a set of disks (i.e. we wanted to list the free spaces, partitions, filesystems, etc. at some point of the whole devicegraph).
We came up with the idea of a Devicegraph query object that is already partially implemented and seems to solve our problems. But if we are going to start using it in many places (there will be calls in almost every single test) I want to make sure the API does not suck much. I have two alternatives and would like to hear/read opinions.
As usual, we don't want to over-engineer it, but we don't want to throw it away with the next requirement change either.
Explanation by example, hopefully enough (ping me if it's not). Please read both options before commenting (I actually like more the second one).
Option 1 - Only a DevicegraphQuery class
https://gist.github.com/ancorgs/9c628ef0f4fa717a2817
But, what about something like this? more_fine_query.disks
With the implementation I have in mind, it would return the same that fine_query.disks, but some people could expect it to return only disks from fine_query.disks which, as additional requirement, contain any primary partition.
This, of course, can be solved documenting which restrictions are honored by each query method (for example, #disks would only honor #with_disk while #partitions would honor both #with_disk and #with_partition).
That is, the expected direction of the hierarchy (like "a filesystem can be expected at some point below a disk, but not the other way around") would be in the source code and documentation of the methods at DevicegraphQuery.
Option 2 - A WhateverQuery class per type of object
https://gist.github.com/ancorgs/a98597fa9a5e348eaaac
This is my preferred option for several reasons, even if it means implementing more classes (something that is not bad, per se).
First of all, the direction of the hierarchy is reflected in the API (DiskQuery has a method #filesystems which returns a FilesystemQuery but not the other way around).
It also makes possible (and easy) to write specific filters for a given type of query, like # Get an array of free spaces big enough to be used for something FreeDiskSpaceQuery.new(devicegraph).useful.to_a
I also find it more readable for the tests and very Ruby-like (even Rails-like), but that can be a matter of taste.
Opinions?
Hi, I check both examples and few notes:
1) we should reinvent wheel. ActiveModel already have queries, so why not use it?
2) Why we need to create query object? It is not intuitive for me. I expect query is action on container, not object itself. It is same for me like having `Question.new(human).age?` versus `human.age?` Also in ActiveModel mentioned above you do not use something like `Query.new(Human.all).with_age(25)` but `Human.all.with_age(25)`
3) it is probably related to 2), second approach looks better to me, just please no query object. Object should be containers like Disks or Partitions holding object of given type which you can query, filter, whatever. See for example this code using this approach in yast(!) - https://github.com/yast/yast-cio/blob/master/src/lib/iochannel/channels.rb#L...
So to sum it, for me the main design issue is that query is not object, but action. Object is for me container holding objects of given type with actions on it. Then you can have your code like: https://gist.github.com/jreidinger/3883aa68662e9a7fdc7b
just my 2c Josef
On 03/21/2016 09:52 AM, Josef Reidinger wrote:
On Fri, 18 Mar 2016 09:57:11 +0100 Ancor Gonzalez Sosa ancor@suse.de wrote:
I have a design decision to take and I need advise. Please, stay on topic (i.e. "design a ruby layer to query the not-under-discussion structure of devicegraphs coming from libstorage").
While writing the code for the new proposal we needed to query a devicegraph (hopefully you all know what a devicegraph is in the libstorage-ng world) for things like:
- all free spaces in a given subset of disks (candidates for install)
- all filesystems in the same subset, not caring if they are directly
on the disk or on a partition table
While writing tests, we found we had similar requirements but without constraining those queries to a set of disks (i.e. we wanted to list the free spaces, partitions, filesystems, etc. at some point of the whole devicegraph).
We came up with the idea of a Devicegraph query object that is already partially implemented and seems to solve our problems. But if we are going to start using it in many places (there will be calls in almost every single test) I want to make sure the API does not suck much. I have two alternatives and would like to hear/read opinions.
As usual, we don't want to over-engineer it, but we don't want to throw it away with the next requirement change either.
Explanation by example, hopefully enough (ping me if it's not). Please read both options before commenting (I actually like more the second one).
Option 1 - Only a DevicegraphQuery class
https://gist.github.com/ancorgs/9c628ef0f4fa717a2817
But, what about something like this? more_fine_query.disks
With the implementation I have in mind, it would return the same that fine_query.disks, but some people could expect it to return only disks from fine_query.disks which, as additional requirement, contain any primary partition.
This, of course, can be solved documenting which restrictions are honored by each query method (for example, #disks would only honor #with_disk while #partitions would honor both #with_disk and #with_partition).
That is, the expected direction of the hierarchy (like "a filesystem can be expected at some point below a disk, but not the other way around") would be in the source code and documentation of the methods at DevicegraphQuery.
Option 2 - A WhateverQuery class per type of object
https://gist.github.com/ancorgs/a98597fa9a5e348eaaac
This is my preferred option for several reasons, even if it means implementing more classes (something that is not bad, per se).
First of all, the direction of the hierarchy is reflected in the API (DiskQuery has a method #filesystems which returns a FilesystemQuery but not the other way around).
It also makes possible (and easy) to write specific filters for a given type of query, like # Get an array of free spaces big enough to be used for something FreeDiskSpaceQuery.new(devicegraph).useful.to_a
I also find it more readable for the tests and very Ruby-like (even Rails-like), but that can be a matter of taste.
Opinions?
Hi, I check both examples and few notes:
- we should reinvent wheel. ActiveModel already have queries, so why
not use it?
Actually not. ActiveRecord provides queries. ActiveModel provides a lot of useful stuff as the base for ActiveRecord, but not the queries themselves.
Moreover, the second alternative presented in my mail is heavily inspired by ActiveRecord query interface.
Using ActiveModel itself (instead of just taking it as inspiration) is an interesting idea, but Disk, Partition and so on are classes already provided by the underlying libstorage, so I don't want to mess to much with them including ActiveModel modules and so on.
- Why we need to create query object? It is not intuitive for me. I
expect query is action on container, not object itself. It is same for me like having `Question.new(human).age?` versus `human.age?` Also in ActiveModel mentioned above you do not use something like `Query.new(Human.all).with_age(25)` but `Human.all.with_age(25)`
Actually, in ActiveRecord you also have a separate ActiveRecord::Relation class, very similar in purpose to the query classes of the second alternative.
Your example in ActiveRecord can broken into pieces like this, a_relation_object = Human.all # ActiveRecord::Relation a_relation_object.with_age(25)
So the separate query class is there and you are interacting with it when doing "with_age(25)". Pretty much the same that is proposed in my mail. The only difference if that ActiveRecord provides a module which adds class methods like .all to your models in order to "hide" the creation of those ActiveRecord::Relation objects.
We can easily do a similar thing implementing the second alternative and then adding a refinement like refine ::Storage::Devicegraph.singleton_class do def disks DevicegraphQuery.new(self).disks end def filesystems DevicegraphQuery.new(self).filesystems end ... end
- it is probably related to 2), second approach looks better to me,
just please no query object. Object should be containers like Disks or Partitions holding object of given type which you can query, filter, whatever. See for example this code using this approach in yast(!) - https://github.com/yast/yast-cio/blob/master/src/lib/iochannel/channels.rb#L...
Indeed, DiskQuery, FilesystemQuery and so on in my second proposal are containers. Pretty much the same you expect, apart from the name.
So to sum it, for me the main design issue is that query is not object, but action. Object is for me container holding objects of given type with actions on it. Then you can have your code like: https://gist.github.com/jreidinger/3883aa68662e9a7fdc7b
As said, what you suggest is exactly option 2 with the refinement I wrote two paragraph ago. You simply call my XxxQuery classes as "array on steroid". :-) So simply renaming DiskQuery to DiskList or DiskArrayOnSteroids will 100% fit your expectations.
So I will count you as a vote for option 2. ;-)
Cheers.
On 21.03.2016 09:52, Josef Reidinger wrote:
- we should reinvent wheel. ActiveModel already have queries, so why
not use it?
Because it's Rails, and it would drag in a lot of more dependencies and a steep learning curve for everybody who is not already familiar with it?
Let's please keep this simple and as self-sufficient as possible.
Kind regards
On Mon, 21 Mar 2016 11:23:53 +0100 Stefan Hundhammer shundhammer@suse.de wrote:
On 21.03.2016 09:52, Josef Reidinger wrote:
- we should reinvent wheel. ActiveModel already have queries, so
why not use it?
Because it's Rails, and it would drag in a lot of more dependencies and a steep learning curve for everybody who is not already familiar with it?
Let's please keep this simple and as self-sufficient as possible.
Kind regards
Ah, I confuse too much people. I mean why not use its API. I do not mean to use it as library. AR API is quite intuitive for majority of ruby developers.
Josef
On Fri, 2016-03-18 at 09:57 +0100, Ancor Gonzalez Sosa wrote:
I have a design decision to take and I need advise. Please, stay on topic (i.e. "design a ruby layer to query the not-under-discussion structure of devicegraphs coming from libstorage").
While writing the code for the new proposal we needed to query a devicegraph (hopefully you all know what a devicegraph is in the libstorage-ng world) for things like:
- all free spaces in a given subset of disks (candidates for install)
- all filesystems in the same subset, not caring if they are directly
on the disk or on a partition table
While writing tests, we found we had similar requirements but without constraining those queries to a set of disks (i.e. we wanted to list the free spaces, partitions, filesystems, etc. at some point of the whole devicegraph).
We came up with the idea of a Devicegraph query object that is already partially implemented and seems to solve our problems. But if we are going to start using it in many places (there will be calls in almost every single test) I want to make sure the API does not suck much. I have two alternatives and would like to hear/read opinions.
As usual, we don't want to over-engineer it, but we don't want to throw it away with the next requirement change either.
Explanation by example, hopefully enough (ping me if it's not). Please read both options before commenting (I actually like more the second one).
Option 1 - Only a DevicegraphQuery class
https://gist.github.com/ancorgs/9c628ef0f4fa717a2817
But, what about something like this? more_fine_query.disks
With the implementation I have in mind, it would return the same that fine_query.disks, but some people could expect it to return only disks from fine_query.disks which, as additional requirement, contain any primary partition.
This, of course, can be solved documenting which restrictions are honored by each query method (for example, #disks would only honor #with_disk while #partitions would honor both #with_disk and #with_partition).
That is, the expected direction of the hierarchy (like "a filesystem can be expected at some point below a disk, but not the other way around") would be in the source code and documentation of the methods at DevicegraphQuery.
Option 2 - A WhateverQuery class per type of object
https://gist.github.com/ancorgs/a98597fa9a5e348eaaac
This is my preferred option for several reasons, even if it means implementing more classes (something that is not bad, per se).
First of all, the direction of the hierarchy is reflected in the API (DiskQuery has a method #filesystems which returns a FilesystemQuery but not the other way around).
It also makes possible (and easy) to write specific filters for a given type of query, like # Get an array of free spaces big enough to be used for something FreeDiskSpaceQuery.new(devicegraph).useful.to_a
I also find it more readable for the tests and very Ruby-like (even Rails-like), but that can be a matter of taste.
Opinions?
I'd go for the second approach because it's more readable and it can be extended in a easier way.
Regards, Imo
-
Ancor Gonzalez Sosa -
Imobach González Sosa -
Josef Reidinger -
Stefan Hundhammer