On Mon, 2011-05-02 at 13:40 +0200, [email protected] wrote:
> From: Michal Fojtik <[email protected]>
ACK after addressing the comments below:
> diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> index 4b9899a..fc9ab73 100644
> --- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> +++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
> @@ -509,6 +509,56 @@ module Deltacloud
> end
> end
>
> + def addresses(credentials, opts={})
> + ec2 = new_client(credentials)
> + address_id = (opts and opts[:id]) ? opts[:id] : []
Shouldn't address_id always be an array, i.e.,
address_id = (opts and opts[:id]) ? [opts[:id]] : []
Also a request for http://localhost:3001/api/addresses?id=127.0.0.1
leads to a backend error (error code 502)
It would be more natural to me if that just resulted in an empty list.
A request for http://localhost:3001/api/addresses/127.0.0.1 also leads
to a 502; that should be a 404
> diff --git a/server/server.rb b/server/server.rb
> index dcc76e5..344d4bf 100644
> --- a/server/server.rb
> +++ b/server/server.rb
> @@ -858,3 +858,89 @@ collection :buckets do
> end
>
> end
> +
> +get '/api/addresses/:id/associate' do
> + @instances = driver.instances(credentials)
> + @address = Address::new(:id => params[:id])
> + respond_to do |format|
> + format.html { haml :"addresses/associate" }
> + end
> +end
> +
> +collection :addresses do
> + description "Manage IP addresses"
> +
> + operation :index do
> + description "List IP addresses assigned to your account."
> + with_capability :addresses
> + control do
> + filter_all :addresses
> + end
> + end
> +
> + operation :show do
> + description "Show details about IP addresses specified by given ID"
> + with_capability :address
> + param :id, :string, :required
> + control { show :address }
> + end
> +
> + operation :create do
> + description "Acquire a new IP address for use with your account."
> + with_capability :create_address
> + control do
> + @address = driver.create_address(credentials, {})
> + respond_to do |format|
> + format.html { haml :"addresses/show" }
> + format.xml do
> + response.status = 201 # Created
Also set the Location header to the URL for the new address.
> diff --git a/server/views/addresses/index.xml.haml
> b/server/views/addresses/index.xml.haml
> new file mode 100644
> index 0000000..e2a564f
> --- /dev/null
> +++ b/server/views/addresses/index.xml.haml
> @@ -0,0 +1,4 @@
> +!!!XML
> +%addresses
> + - @elements.each do |c|
> + = haml :'address/show', :locals => { :@address => c, :partial => true }
Typo:
^^^^ addresses/show
> diff --git a/server/views/addresses/show.xml.haml
> b/server/views/addresses/show.xml.haml
> new file mode 100644
> index 0000000..a450690
> --- /dev/null
> +++ b/server/views/addresses/show.xml.haml
> @@ -0,0 +1,12 @@
> +- unless defined?(partial)
> + !!! XML
> +%address{ :href => address_url(@address.id), :id => @address.id }
We should provide the IP as a separate attribute; there's no need to
cement ID == IP in the API.
David
