[Ovirt-devel] [PATCH 2/5] API for storage pools

[David Lutterkort]

On Thu, 2008-08-07 at 13:22 -0400, Scott Seago wrote:
> David Lutterkort wrote:
> > - List and filter storage pools by ipaddr, path, target and hardware_pool_id
> > - Show an individual storage pool
> > - Create a new storage pool
> > - Destroy a storage pool
> > ---
> >   
> I'll mention the obvious comment that we've got to filter all this stuff 
> by permissions (applies to all patches).

Yes, that's one of the reasons why I would like to serve WUI and API out
of the same controllers.

> > +    def create
> > +        # Somehow the attribute 'type' never makes it through
> > +        # Maybe because there is a (deprecated) Object.type ?
> >   
> Yes, ActiveRecord has docs that address this. You have to say foo[:type] 
> rather than foo.type !

The issue was that 'type' never showed up on the other side, i.e.
ActiveResource did not even send it as part of the request.

> We've run into the same problem with target. foo[:target] rather than 
> foo.target
> 
> I'm not sure how this affects the API though -- other than making sure 
> we set/retrieve type and  target attributes correctly.
> > +        pool = params[:storage_pool]
> > +        type = pool[:storage_type]
> > +        pool.delete(:storage_type)
> > +        @storage_pool = StoragePool.factory(type, pool)
> > +        respond_to do |format|
> > +            if @storage_pool
> > +                if @storage_pool.save
> >   
> We've tried to be consistent in using save! and dealing with errors in 
> rescue blocks, as it's too easy to let error returns fall through this 
> way. In addition, for multi-object transactions, this lets us deal with 
> errors in one place for  the whole transaction.

Ok .. makes sense

David