On 03/01/2012 01:09 PM, Jan Provaznik wrote:
On 02/24/2012 01:21 PM, mtaylor@redhat.com wrote:
From: Martyn Taylormtaylor@redhat.com
lib/aeolus_cli.rb | 2 ++ lib/aeolus_cli/command/base_command.rb | 28 ++++++++++++++++++++-------- lib/aeolus_cli/exceptions.rb | 28 ++++++++++++++++++++++++++++ spec/command/base_command_spec.rb | 5 +++++ spec/spec_helper.rb | 1 + 5 files changed, 56 insertions(+), 8 deletions(-) create mode 100644 lib/aeolus_cli/exceptions.rb
diff --git a/lib/aeolus_cli.rb b/lib/aeolus_cli.rb index 8070845..2e6e88c 100644 --- a/lib/aeolus_cli.rb +++ b/lib/aeolus_cli.rb @@ -33,3 +33,5 @@ require File.join(File.dirname(__FILE__), 'aeolus_cli/model', 'provider') require File.join(File.dirname(__FILE__), 'aeolus_cli/model', 'provider_account') require File.join(File.dirname(__FILE__), 'aeolus_cli/model', 'provider_type') require File.join(File.dirname(__FILE__), 'aeolus_cli/model', 'environment')
+require File.join(File.dirname(__FILE__), 'aeolus_cli/', 'exceptions') diff --git a/lib/aeolus_cli/command/base_command.rb b/lib/aeolus_cli/command/base_command.rb index 422e5f4..b9584d6 100644 --- a/lib/aeolus_cli/command/base_command.rb +++ b/lib/aeolus_cli/command/base_command.rb @@ -24,11 +24,15 @@ module Aeolus attr_accessor :options
def initialize(opts={}, logger=nil)
logger(logger)
@options = opts
@config_location = "~/.aeolus-cli"
@config = load_config
configure_active_resource
begin
logger(logger)
@options = opts
@config_location = "~/.aeolus-cli"
@config = load_config
configure_active_resource
rescue => e
handle_exception(e)
end end protected
@@ -115,6 +119,10 @@ module Aeolus code = "Argument Error" message = e.message
elsif e.is_a?(Aeolus::CLI::ConfigError)
code = "Config Error"
message = e.message + "; please check ~/.aeolus-cli"
elsif e.respond_to?(:response) doc = Nokogiri::XML e.response.body code = doc.xpath("/error/code").text
@@ -167,9 +175,13 @@ module Aeolus end
def configure_active_resource
Aeolus::CLI::Base.site = @config[:conductor][:url]
Aeolus::CLI::Base.user = @config[:conductor][:username]
Aeolus::CLI::Base.password = @config[:conductor][:password]
if @config.has_key?(:conductor)&&
@config[:conductor].keys.to_set.subset?([:url, :password, :username].to_set)
Aeolus::CLI::Base.site = @config[:conductor][:url]
Aeolus::CLI::Base.user = @config[:conductor][:username]
Aeolus::CLI::Base.password = @config[:conductor][:password]
else
raise Aeolus::CLI::ConfigError.new("Error in configuration
file")
end
If I comment out :url key, so it looks: :conductor: #:url: http://localhost:3000/api :username: admin :password: password
then I get: $ aeolus-image list --images
ERROR: Unknown Error => undefined method `path' for nil:NilClass /usr/lib/ruby/gems/1.8/gems/activeresource-3.0.10/lib/active_resource/base.rb:562:in `prefix' ...
Could this be extended to print which config key is missing? Something like this:
raise Aeolus::CLI::ConfigError.new("#{key} is missing") unless @config.has_key?(:conductor) [:url, :password, :username].each do |key| raise Aeolus::CLI::ConfigError.new("#{key} is missing") unless @config[:conductor].has_key?(key) end
Good Idea I've updated and resent
end # Print Collection to STDOUT in a column layout
diff --git a/lib/aeolus_cli/exceptions.rb b/lib/aeolus_cli/exceptions.rb new file mode 100644 index 0000000..a523266 --- /dev/null +++ b/lib/aeolus_cli/exceptions.rb @@ -0,0 +1,28 @@ +# +# Copyright 2011 Red Hat, Inc. +# +# Licensed 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 Aeolus
- module CLI
- class Error< StandardError
attr_reader :message
def initialize(message)
@message = message
end
- end
Any reason for Error definition? Isn't sufficient this?
class ConfigError< StandardError; end
Yes, Aeouls::CLI::Error is the default Error within this module. It also overrides the message accessor and the constructor to only allow messages created with message, and set this to readable only.
In the future we might extend Aeolus::CLI::Error to have an Error code.
- class ConfigError< Error; end
- end
+end \ No newline at end of file diff --git a/spec/command/base_command_spec.rb b/spec/command/base_command_spec.rb index 8c1ef5f..f71f91a 100644 --- a/spec/command/base_command_spec.rb +++ b/spec/command/base_command_spec.rb @@ -26,6 +26,11 @@ module Aeolus Aeolus::CLI::Base.user.to_s.should == "admin" Aeolus::CLI::Base.password.to_s.should == "password" end
it "should raise error on invalid config" do
base_command.instance_eval { @config = {} }
lambda { base_command.send(:configure_active_resource)
}.should raise_error(Aeolus::CLI::ConfigError, "Error in configuration file")
end end describe "#read_file" do
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index d198732..368c18f 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -14,6 +14,7 @@
$:<< File.expand_path(File.join(File.dirname(__FILE__), "../lib/aeolus_cli/command/")) $:<< File.expand_path(File.join(File.dirname(__FILE__), "../lib/aeolus_cli/model/")) +$:<< File.expand_path(File.join(File.dirname(__FILE__), "../lib/aeolus_cli/"))
require 'aeolus_cli' require 'stringio'
Hi Martyn, couple of minor notes inline.
Jan
Thanks
Martyn