On Sep 13, 2012, at 10:18 PM, David Lutterkort <[email protected]> wrote:
Hi, Also I found some nasty race-conditions that can occur when two clients override the Deltacloud.config[]. I have something in progress to make this more nice :) -- Michal > On Wed, 2012-09-12 at 15:32 +0300, [email protected] wrote: >> From: marios <[email protected]> >> >> related to https://issues.apache.org/jira/browse/DTACLOUD-316 >> https://github.com/ruby-openstack/ruby-openstack/pull/13 >> >> Signed-off-by: marios <[email protected]> >> --- >> .../deltacloud/drivers/openstack/openstack_driver.rb | 19 >> +++++++++++++------ >> server/lib/deltacloud/server.rb | 6 +++++- >> 2 files changed, 18 insertions(+), 7 deletions(-) >> >> diff --git a/server/lib/deltacloud/drivers/openstack/openstack_driver.rb >> b/server/lib/deltacloud/drivers/openstack/openstack_driver.rb >> index 5b29840..ca824bd 100644 >> --- a/server/lib/deltacloud/drivers/openstack/openstack_driver.rb >> +++ b/server/lib/deltacloud/drivers/openstack/openstack_driver.rb >> @@ -40,6 +40,19 @@ module Deltacloud >> >> define_hardware_profile('default') >> >> + def respond_to?(capability)#, credentials) >> + if capability == :buckets >> + begin >> + client = new_client(Deltacloud.config["openstack_creds"], >> capability) >> + rescue Deltacloud::ExceptionHandler::NotImplemented >> #OpenStack::Exception::NotImplemented... >> + return false >> + end >> + return true >> + else >> + super(capability) >> + end >> + end > > This is not a good idea - it means that something as innocuous looking > as driver.respond_to?(:buckets) has major sideeffects and fail for all > kinds of reasons. > > The right fix is to change the supported_collections method in > deltacloud_helper.rb - this whol capability checking seems way too > convoluted to me, anyway. Why can't we have a > supported_collections(credentials) method in the base driver which, for > all I care, use introspection to figure out which collections are > available, and override that in the OpenStack driver ? > >> def hardware_profiles(credentials, opts = {}) >> os = new_client(credentials) >> results = [] >> @@ -351,12 +364,6 @@ private >> end >> end >> >> - def cloudfiles_client(credentials) >> - safely do >> - CloudFiles::Connection.new(:username => credentials.user, >> :api_key => credentials.password) >> - end >> - end >> - >> #NOTE: for the convert_from_foo methods below... openstack-compute >> #gives Hash for 'flavors' but OpenStack::Compute::Flavor for 'flavor' >> #hence the use of 'send' to deal with both cases and save duplication >> diff --git a/server/lib/deltacloud/server.rb >> b/server/lib/deltacloud/server.rb >> index c7c6c41..4b9485e 100644 >> --- a/server/lib/deltacloud/server.rb >> +++ b/server/lib/deltacloud/server.rb >> @@ -44,8 +44,12 @@ module Deltacloud >> set :config, Deltacloud[:deltacloud] >> >> get '/' do >> - if params[:force_auth] >> + if driver.name == "openstack" or params[:force_auth] > > Why is this check necessary ? The LazyCredentials helper already throws > a 401 if you try to use credentials and they are not provided. > >> return [401, 'Authentication failed'] unless >> driver.valid_credentials?(credentials) >> + if driver.name == "openstack" >> + Deltacloud.config["openstack_creds"] = credentials > > You can't do that - that's a global object shared across all threads. > >> + #or here also works: Thread.current["openstack_creds"] = >> credentials > > This would be better. Though even better would be a direct > supported_collections(credentials) method on the drivers. > > David > >
