Ovirt devel - Apr 2008 - [Patch] refactor Pool models to use nested sets

Refactor Pool models (Hardware and VM) to implement the nested set 
model. As part of this I've refactored the way the various daemon 
processes include the model code and connect to the database, since we 
also have to include the betternestedset plugin code. Now all non-rails 
services that need to access the ovirt models just need to include 
dutils/active_record_env.rb -- this will include the nested set stuff, 
connect to the db, and include all of the ovirt models.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: nested-set-refactor.patch
Type: text/x-patch
Size: 33214 bytes
Desc: not available
URL:
<http://listman.redhat.com/archives/ovirt-devel/attachments/20080421/adaf1b1d/attachment.bin>

On Mon, 2008-04-21 at 09:44 -0400, Scott Seago wrote:
> Refactor Pool models (Hardware and VM) to implement the nested set 
> model. As part of this I've refactored the way the various daemon 
> processes include the model code and connect to the database, since we 
> also have to include the betternestedset plugin code. Now all non-rails 
> services that need to access the ovirt models just need to include 
> dutils/active_record_env.rb -- this will include the nested set stuff, 
> connect to the db, and include all of the ovirt models.
> _______________________________________________
> Ovirt-devel mailing list
> Ovirt-devel at redhat.com
> https://www.redhat.com/mailman/listinfo/ovirt-devel
Looks fine to me overall, with a couple of thoughts/comments/questions:
*  In HardwareController
  +    @parent = Pool.find(params[:parent_id])
  +    @perm_obj = @parent
  should we check for the param?  What if it is null?  looks like the
next @perm_obj depends on it being there (this applies in multiple
places actually)
*  Why reverse_each in Pool here rather than just each?
  +    self_and_ancestors.reverse_each do |the_pool|
*  Why do we have some views that are .rthml and others that
are .html.erb?
  diff --git a/wui/src/app/views/dashboard/index.html.erb
b/wui/src/app/views/dashboard/index.html.erb
*  Is there is way we can specify this path w/o being absolute?  Maybe a
setable env var or maybe use gems for our resuable components?  And what
if the wui is on a different box than the daemons, since we were
planning to split them out into their own rpm(I thought)?
  diff --git a/wui/src/dutils/active_record_env.rb
b/wui/src/dutils/active_record_env.rb
  +$: << File.join(File.dirname(__FILE__), "../app")
  +$: << File.join(File.dirname(__FILE__),
"../vendor/plugins/betternestedset/lib")
  +require '/usr/share/ovirt-wui/vendor/plugins/betternestedset/init.rb'
  +
  +def database_connect
  +  $dbconfig
YAML::load(ERB.new(IO.read('/usr/share/ovirt-wui/config/database.yml')).result)


So I guess really only the first is a possible enhancement for right
now, the rest are just things to consider.