Mailing-List: contact deltacloud-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: deltacloud-dev@incubator.apache.org
Received-SPF: pass (athena.apache.org: domain of lutter@redhat.com designates
 209.132.183.28 as permitted sender)
Subject: Re: [PATCH core] Allow setting the provider on the server side,
 and implement region overriding in the ec2 driver based off of the provider
 (rev 2).
From: David Lutterkort <lutter@redhat.com>
To: deltacloud-dev@incubator.apache.org
In-Reply-To: <1291986394-10944-1-git-send-email-tcrawley@redhat.com>
References: <1291986394-10944-1-git-send-email-tcrawley@redhat.com>
Content-Type: text/plain; charset="ISO-8859-15"
Organization: Red Hat Inc
Date: Tue, 14 Dec 2010 17:00:01 -0800
Message-ID: <1292374801.4244.146.camel@avon.watzmann.net>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit

On Fri, 2010-12-10 at 08:06 -0500, tcrawley@redhat.com wrote:
> From: Tobias Crawley <tcrawley@redhat.com>
> 
> The provider can now be set on the commandline to deltacloudd via -P/--provider
> (putting it in ENV['API_PROVIDER']), and also from the client via a header
> (putting it in Thread.current[:provider]).
> 
> The ec2 driver is currently the only driver that understands providers, and
> looks in Thread.current[:provider], then ENV['API_PROVIDER'], then falls back
> to a default (currently 'us-east-1', the AWS default).

Sorry for being late to the party; the word 'provider' seems very
unclear to me. In addition, the driver does some scary URL surgery that
seems less than robust to me.

I'd prefer if we rename this mechanism into 'endpoint' (and
ENV['API_ENDPOINT']) with the expectation that the entire URL that we
should talk to is passed in. With that, it also becomes possible to use
the EC2 driver against a private Eucalyptus instance.

> +  def endpoint_for_service(service)
> +    url = ""
> +    url << case service
> +           when :ec2
> +             'ec2.'
> +           when :elb
> +             'elasticloadbalancing.'
> +           end
> +    url << (Thread.current[:provider] || ENV['API_PROVIDER'] || DEFAULT_REGION)
> +    url << '.amazonaws.com'
> +    url
> +  end

This is the code I object to; and as I said, the URL surgery restricts
unnecessarily how the EC2 driver can be used.

David