deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From David Lutterkort <lut...@redhat.com>
Subject Re: [PATCH core 1/2] EC2: Added initial support for EC2 frontend
Date Thu, 10 May 2012 22:06:52 GMT
On Thu, 2012-05-10 at 17:28 +0200, mfojtik@redhat.com wrote:

> diff --git a/server/lib/ec2/helpers.rb b/server/lib/ec2/helpers.rb
> new file mode 100644
> index 0000000..32380fa
> --- /dev/null
> +++ b/server/lib/ec2/helpers.rb
> @@ -0,0 +1,29 @@
> +
> +require_relative '../deltacloud/helpers/driver_helper'
> +require_relative '../deltacloud/helpers/auth_helper'
> +require_relative '../deltacloud/core_ext/string'
> +require_relative '../deltacloud/core_ext/array'
> +require_relative '../deltacloud/core_ext/hash'
> +require_relative '../deltacloud/core_ext/integer'
> +require_relative '../deltacloud/core_ext/proc'

I wonder if we shouldn't just load that all in config.ru (or have a
general file that every frontend requires)

> diff --git a/server/lib/ec2/helpers/convertor.rb b/server/lib/ec2/helpers/convertor.rb
> new file mode 100644
> index 0000000..9d98e5b
> --- /dev/null
> +++ b/server/lib/ec2/helpers/convertor.rb
> @@ -0,0 +1,140 @@
> +# Licensed to the Apache Software Foundation (ASF) under one or more
> +# contributor license agreements.  See the NOTICE file distributed with
> +# this work for additional information regarding copyright ownership.  The
> +# ASF licenses this file to you under the Apache License, Version 2.0 (the
> +# "License"); you may not use this file except in compliance with the
> +# License.  You may obtain a copy of the License at
> +#
> +#       http://www.apache.org/licenses/LICENSE-2.0
> +#
> +# Unless required by applicable law or agreed to in writing, software
> +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
> +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
> +# License for the specific language governing permissions and limitations
> +# under the License.
> +
> +module Deltacloud::EC2
> +
> +  class Convertor
> +
> +    def self.convert(builder, action, result)
> +      klass_name = ActionHandler::MAPPINGS[action][:method].to_s.camelize
> +      klass = Convertor.const_get(klass_name)
> +      klass.new(builder, result).to_xml
> +    end

Are you sure this is simpler than explicit HAML templates to generate
XML ? Since we are storing the driver result in a model object anyway, a
template might be clearer. (Minor nit: it's converter rather than
convertor)

> diff --git a/server/lib/ec2/query_parser.rb b/server/lib/ec2/query_parser.rb
> new file mode 100644
> index 0000000..53c7d61
> --- /dev/null
> +++ b/server/lib/ec2/query_parser.rb
> @@ -0,0 +1,129 @@
> +# Licensed to the Apache Software Foundation (ASF) under one or more
> +# contributor license agreements.  See the NOTICE file distributed with
> +# this work for additional information regarding copyright ownership.  The
> +# ASF licenses this file to you under the Apache License, Version 2.0 (the
> +# "License"); you may not use this file except in compliance with the
> +# License.  You may obtain a copy of the License at
> +#
> +#       http://www.apache.org/licenses/LICENSE-2.0
> +#
> +# Unless required by applicable law or agreed to in writing, software
> +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
> +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
> +# License for the specific language governing permissions and limitations
> +# under the License.
> +
> +module Deltacloud::EC2
> +
> +  class ActionHandler
> +
> +    MAPPINGS = {
> +      :describe_availability_zones => { :method => :realms, :params => { 'ZoneName.1'
=> :id } },
> +      :describe_images => { :method => :images, :params => { 'ImageId.1' =>
:id }},
> +      :describe_instances => { :method => :instances, :params => {} },
> +      :run_instances => { :method => :create_instance, :params => { 'ImageId'
=> :image_id, 'InstanceType' => :hwp_id, 'Placement.AvailabilityZone' => :realm_id
}}
> +    }

This is screaming "invent a DSL for me" ;) But I'd hold off until we
have a PoC that's fully working across a few drivers.

> +    def initialize(request_id, params={})
> +      @request_id = request_id
> +      @action = (params.delete('Action') || '').underscore.intern

You can't intern an empty string - not having an Action should produce
an error anyway.

David



Mime
View raw message