Mailing-List: contact dev-help@deltacloud.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@deltacloud.apache.org
Received-SPF: pass (nike.apache.org: domain of mfojtik@redhat.com designates
 209.132.183.28 as permitted sender)
Subject: Re: [PATCH 4/5] Adds mocking of CIMI::Network to deltacloud mock
 driver/client
Mime-Version: 1.0 (Apple Message framework v1251.1)
Content-Type: text/plain; charset=us-ascii
From: Michal Fojtik <mfojtik@redhat.com>
In-Reply-To: <1326477506-16989-5-git-send-email-marios@redhat.com>
Date: Mon, 16 Jan 2012 13:44:27 +0100
Content-Transfer-Encoding: quoted-printable
Message-Id: <0FC28C8F-FDBE-456C-BE4D-945C5D959B05@redhat.com>
References: <1326477506-16989-1-git-send-email-marios@redhat.com>
 <1326477506-16989-5-git-send-email-marios@redhat.com>
To: dev@deltacloud.apache.org

On Jan 13, 2012, at 6:58 PM, marios@redhat.com wrote:

ACK.

Minor nits inline.

  -- Michal


> From: marios <marios@redhat.com>
>=20
>=20
> Signed-off-by: marios <marios@redhat.com>
> ---
> server/lib/cimi/model/network.rb                  |    4 ++--
> server/lib/deltacloud/drivers/mock/mock_client.rb |   18 =
++++++++++++++++++
> server/lib/deltacloud/drivers/mock/mock_driver.rb |   18 =
++++++++++++++++++
> 3 files changed, 38 insertions(+), 2 deletions(-)
>=20
> diff --git a/server/lib/cimi/model/network.rb =
b/server/lib/cimi/model/network.rb
> index d46f244..08fcd97 100644
> --- a/server/lib/cimi/model/network.rb
> +++ b/server/lib/cimi/model/network.rb
> @@ -44,9 +44,9 @@ class CIMI::Model::Network < CIMI::Model::Base
>   def self.find(id, context)
>     networks=3D[]
>     if id=3D=3D:all
> -#FIXME
> +      networks =3D context.driver.networks(context.credentials, =
{:env=3D>context})
>     else
> -#FIXME
> +      networks =3D context.driver.networks(context.credentials, =
{:id=3D>id, :env=3D>context})
>     end
>     networks
>   end
> diff --git a/server/lib/deltacloud/drivers/mock/mock_client.rb =
b/server/lib/deltacloud/drivers/mock/mock_client.rb
> index 96157cd..325caab 100644
> --- a/server/lib/deltacloud/drivers/mock/mock_client.rb
> +++ b/server/lib/deltacloud/drivers/mock/mock_client.rb
> @@ -91,6 +91,24 @@ module Deltacloud::Drivers::Mock
>       FileUtils.rm(fname) if File::exists?(fname)
>     end
>=20
> +    def load_all_cimi(model_name)
> +        model_files =3D Dir[File::join(cimi_dir(model_name), =
"*.json")]
> +        model_files.map{|f| IO.read(f)}

You can also use: File.read(f). I think it would be better to return an
'File' exception when reading fails than IO error :)

> +    end
> +
> +    def load_cimi(model_name, id)
> +        fname=3Dcimi_file(model_name, id)
> +        IO::read(fname)

I would rather use:

IO::read(cimi_file(model_name, id))

to avoid creating a temp variable here (like bellow).

> +    end
> +
> +    def cimi_file(collection, id)
> +      File::join(cimi_dir(collection), "#{id}.json")
> +    end
> +
> +    def cimi_dir(collection)
> +      File::join(@storage_root, "cimi", collection.to_s)
> +    end
> +
>     private
>=20
>     def collection_name(klass)
> diff --git a/server/lib/deltacloud/drivers/mock/mock_driver.rb =
b/server/lib/deltacloud/drivers/mock/mock_driver.rb
> index bee75a0..acabee4 100644
> --- a/server/lib/deltacloud/drivers/mock/mock_driver.rb
> +++ b/server/lib/deltacloud/drivers/mock/mock_driver.rb
> @@ -441,6 +441,17 @@ module Deltacloud::Drivers::Mock
>       return false
>     end
>=20
> +    def networks(credentials, opts=3D{})
> +      check_credentials(credentials)
> +      if opts[:id].nil?
> +        networks =3D @client.load_all_cimi(:network).map{|net| =
CIMI::Model::Network.from_json(net)}
> +        =
networks.map{|net|convert_cimi_network(net,opts[:env])}.flatten
> +      else
> +        network =3D =
CIMI::Model::Network.from_json(@client.load_cimi(:network, opts[:id]))
> +        convert_cimi_network(network, opts[:env])
> +      end
> +    end
> +
>     private
>=20
>     def check_credentials(credentials)
> @@ -477,6 +488,13 @@ module Deltacloud::Drivers::Mock
>       StorageVolume.new(volume)
>     end
>=20
> +    def convert_cimi_network(network, context)
> +      =
uri=3D"#{context.env["rack.url_scheme"]}://#{context.env["HTTP_HOST"]}#{se=
ttings.root_url}/networks/#{network.name}"

If there is a global_collection for :networks then we should have a =
helper:

network_url(network.name)

I think we also have the :absolute option in url_for, if not we should =
consider to add it there.

> +      network.uri=3Duri
> +      network.operations.each{|op| op.href=3Duri}
> +      network
> +    end
> +
>     exceptions do
>=20
>       on /AuthFailure/ do
> --=20
> 1.7.6.5
>=20

------------------------------------------------------
Michal Fojtik, mfojtik@redhat.com
Deltacloud API: http://deltacloud.org