[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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). A few additional comments inline:

 .../controllers/rest/storage_pools_controller.rb   |   60 ++++++++++++++++++++
 wui/src/config/routes.rb                           |    1 +
 2 files changed, 61 insertions(+), 0 deletions(-)
 create mode 100644 wui/src/app/controllers/rest/storage_pools_controller.rb

diff --git a/wui/src/app/controllers/rest/storage_pools_controller.rb b/wui/src/app/controllers/rest/storage_pools_controller.rb
new file mode 100644
index 0000000..99eb158
--- /dev/null
+++ b/wui/src/app/controllers/rest/storage_pools_controller.rb
@@ -0,0 +1,60 @@
+class Rest::StoragePoolsController < ApplicationController
+
+    EQ_ATTRIBUTES = [ :ip_addr, :export_path, :target,
+                      :hardware_pool_id ]
+
+    def index
+        conditions = []
+        EQ_ATTRIBUTES.each do |attr|
+            if params[attr]
+                conditions << "#{attr} = :#{attr}"
+            end
+        end
+
+        @storage = StoragePool.find(:all,
+                     :conditions => [conditions.join(" and "), params],
+                     :order => "id")
+
+        respond_to do |format|
+            format.xml { render :xml => @storage.to_xml }
+        end
+    end
+
+    def show
+        @storage = StoragePool.find(params[:id])
+        respond_to do |format|
+            format.xml { render :xml => @storage.to_xml }
+        end
+    end
+
+    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 ! 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.
+                    format.xml  { render :xml => @storage_pool,
+                        :status => :created,
+                        :location => rest_storage_pool_url(@storage_pool) }
+                else
+                    format.xml  { render :xml => @storage_pool.errors,
+                        :status => :unprocessable_entity }
When we intergrate with the WUI controllers, we should standardize our status/message processing between the REST calls and the AJAX/json calls.
+                end
+            else
+                format.xml  { render :xml => "Illegal storage type #{params[:storage_type]}", :status => :unprocessable_entity }
+            end
+        end
+    end
+
+    def destroy
+        @storage_pool = StoragePool.find(params[:id])
+        @storage_pool.destroy
+        respond_to do |format|
+            format.xml  { head :ok }
+        end
+    end
+end
Scott


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]