This patch updates the previous submission, changes are described below:
- rename imagefactory to iwhd - rename imagefactory-cli to aeolus-cli - fix tests to use fixtures/aeolus-cli - remove class level variables - mv config to sample data - check subcommand is not nil in config parser
From: Martyn Taylor mtaylor@redhat.com
--- .../image_factory/aeolus-image/lib/base_command.rb | 55 ++++++++++++++++++++ .../aeolus-image/sample_data/aeolus-cli | 8 +++ .../aeolus-image/spec/base_command_spec.rb | 24 +++++++++ .../aeolus-image/spec/sample_data/aeolus-cli | 7 +++ .../image_factory/aeolus-image/spec/spec_helper.rb | 9 +++- 5 files changed, 102 insertions(+), 1 deletions(-) create mode 100644 services/image_factory/aeolus-image/sample_data/aeolus-cli create mode 100644 services/image_factory/aeolus-image/spec/base_command_spec.rb create mode 100644 services/image_factory/aeolus-image/spec/sample_data/aeolus-cli
diff --git a/services/image_factory/aeolus-image/lib/base_command.rb b/services/image_factory/aeolus-image/lib/base_command.rb index 4d4fa42..ab93353 100644 --- a/services/image_factory/aeolus-image/lib/base_command.rb +++ b/services/image_factory/aeolus-image/lib/base_command.rb @@ -1,11 +1,17 @@ +require 'yaml' +require 'rest_client' +require 'nokogiri' + module Aeolus module Image #This will house some methods that multiple Command classes need to use. class BaseCommand attr_accessor :options + def initialize(opts={}, logger=nil) logger(logger) @options = opts + @config = load_config end
protected @@ -19,6 +25,55 @@ module Aeolus return @logger end
+ def iwhd + createResource('iwhd') + end + + def conductor + createResource('conductor') + end + + private + def load_config + begin + @config_location = ENV['HOME'] + "/.aeolus-cli" + YAML::load(File.open(@config_location)) + rescue Errno::ENOENT + #TODO: Create a custom exception to wrap CLI Exceptions + raise "Unable to locate configuration file: "" + @config_location + """ + end + end + + def createResource(resource_name) + # Check to see if config has a resource with this name + if !@config.has_key?(resource_name) + raise "Unable to determine resource: " + resource_name + " from configuraion file. Please check: " + @config_location + return + end + + #Use command line arguments for username/password + if @options.has_key?('username') + if @options.has_key?('password') + RestClient::Resource.new(@config[resource_name]['url'], :user => @options['username'], :password => @options['password']) + else + #TODO: Create a custom exception to wrap CLI Exceptions + raise "Password not found for user: " + @options['username'] + end + + #Use config for username/password + elsif @config[resource_name].has_key?('username') + if @config[resource_name].has_key?('password') + RestClient::Resource.new(@config[resource_name]['url'], :user => @config[resource_name]['username'], :password => @config[resource_name]['password']) + else + #TODO: Create a custom exception to wrap CLI Exceptions + raise "Password not found for user: " + @config[resource_name]['username'] + end + + # Do not use authentication + else + RestClient::Resource.new(@config[resource_name]['url']) + end + end end end end diff --git a/services/image_factory/aeolus-image/sample_data/aeolus-cli b/services/image_factory/aeolus-image/sample_data/aeolus-cli new file mode 100644 index 0000000..5b69e21 --- /dev/null +++ b/services/image_factory/aeolus-image/sample_data/aeolus-cli @@ -0,0 +1,8 @@ +# Place this file in: ~/.imagefactory-cli +iwhd: + url: http://localhost:9090 + +conductor: + url: http://localhost:3000/conductor + username: admin + password: password diff --git a/services/image_factory/aeolus-image/spec/base_command_spec.rb b/services/image_factory/aeolus-image/spec/base_command_spec.rb new file mode 100644 index 0000000..69e0697 --- /dev/null +++ b/services/image_factory/aeolus-image/spec/base_command_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper' + +module Aeolus + module Image + describe BaseCommand do + describe BaseCommand do + it "should determine the correct credentials for HTTP Authentication" do + basec = BaseCommand.new + iwhd = basec.send :iwhd + iwhd.user.should == nil + + conductor = basec.send :conductor + conductor.user.should == "admin" + conductor.password.should == "password" + + basec = BaseCommand.new({'username' => "testusername", 'password'=> "testpassword"}) + conductor = basec.send :conductor + conductor.user.should == "testusername" + conductor.password.should == "testpassword" + end + end + end + end +end \ No newline at end of file diff --git a/services/image_factory/aeolus-image/spec/sample_data/aeolus-cli b/services/image_factory/aeolus-image/spec/sample_data/aeolus-cli new file mode 100644 index 0000000..574a233 --- /dev/null +++ b/services/image_factory/aeolus-image/spec/sample_data/aeolus-cli @@ -0,0 +1,7 @@ +iwhd: + url: http://localhost:9090 + +conductor: + url: http://localhost:3000/conductor + username: admin + password: password \ No newline at end of file diff --git a/services/image_factory/aeolus-image/spec/spec_helper.rb b/services/image_factory/aeolus-image/spec/spec_helper.rb index d3adf8d..ad0b5cc 100644 --- a/services/image_factory/aeolus-image/spec/spec_helper.rb +++ b/services/image_factory/aeolus-image/spec/spec_helper.rb @@ -32,4 +32,11 @@ end
Spec::Runner.configure do |config| config.include Helpers -end + config.before(:all) do + Aeolus::Image::BaseCommand.class_eval do + def load_config + YAML::load(File.open(File.join(File.dirname(__FILE__), "sample_data/aeolus-cli"))) + end + end + end +end \ No newline at end of file
On Tue, 2011-06-14 at 15:55 +0100, mtaylor@redhat.com wrote:
From: Martyn Taylor mtaylor@redhat.com
.../image_factory/aeolus-image/lib/base_command.rb | 55 ++++++++++++++++++++ .../aeolus-image/sample_data/aeolus-cli | 8 +++ .../aeolus-image/spec/base_command_spec.rb | 24 +++++++++ .../aeolus-image/spec/sample_data/aeolus-cli | 7 +++ .../image_factory/aeolus-image/spec/spec_helper.rb | 9 +++- 5 files changed, 102 insertions(+), 1 deletions(-) create mode 100644 services/image_factory/aeolus-image/sample_data/aeolus-cli create mode 100644 services/image_factory/aeolus-image/spec/base_command_spec.rb create mode 100644 services/image_factory/aeolus-image/spec/sample_data/aeolus-cli
Couple minor nits inline that can be cleaned up and pushed w/o another review. As discussed in irc, we'll revisit some of our more major differences of opinion in an upcoming sprint, if either of us can convince the other to change their mind :)
diff --git a/services/image_factory/aeolus-image/lib/base_command.rb b/services/image_factory/aeolus-image/lib/base_command.rb index 4d4fa42..ab93353 100644 --- a/services/image_factory/aeolus-image/lib/base_command.rb +++ b/services/image_factory/aeolus-image/lib/base_command.rb @@ -1,11 +1,17 @@ +require 'yaml' +require 'rest_client' +require 'nokogiri'
module Aeolus module Image #This will house some methods that multiple Command classes need to use. class BaseCommand attr_accessor :options
def initialize(opts={}, logger=nil) logger(logger) @options = opts
@config = load_config end protected
@@ -19,6 +25,55 @@ module Aeolus return @logger end
def iwhd
createResource('iwhd')
end
def conductor
createResource('conductor')
end
private
def load_config
begin
@config_location = ENV['HOME'] + "/.aeolus-cli"
YAML::load(File.open(@config_location))
rescue Errno::ENOENT
#TODO: Create a custom exception to wrap CLI Exceptions
raise "Unable to locate configuration file: \"" + @config_location + "\""
end
end
def createResource(resource_name)
You were going to update this method to have a less java-y name ^
# Check to see if config has a resource with this name
if !@config.has_key?(resource_name)
raise "Unable to determine resource: " + resource_name + " from configuraion file. Please check: " + @config_location
Minor spelling error s/configuraion/configuration
return
end
#Use command line arguments for username/password
if @options.has_key?('username')
if @options.has_key?('password')
RestClient::Resource.new(@config[resource_name]['url'], :user => @options['username'], :password => @options['password'])
else
#TODO: Create a custom exception to wrap CLI Exceptions
raise "Password not found for user: " + @options['username']
end
#Use config for username/password
elsif @config[resource_name].has_key?('username')
if @config[resource_name].has_key?('password')
RestClient::Resource.new(@config[resource_name]['url'], :user => @config[resource_name]['username'], :password => @config[resource_name]['password'])
else
#TODO: Create a custom exception to wrap CLI Exceptions
raise "Password not found for user: " + @config[resource_name]['username']
end
# Do not use authentication
else
RestClient::Resource.new(@config[resource_name]['url'])
end
end endend
end diff --git a/services/image_factory/aeolus-image/sample_data/aeolus-cli b/services/image_factory/aeolus-image/sample_data/aeolus-cli new file mode 100644 index 0000000..5b69e21 --- /dev/null +++ b/services/image_factory/aeolus-image/sample_data/aeolus-cli @@ -0,0 +1,8 @@ +# Place this file in: ~/.imagefactory-cli
Update directions to have correct path ^
+iwhd:
+conductor:
- url: http://localhost:3000/conductor
- username: admin
- password: password
diff --git a/services/image_factory/aeolus-image/spec/base_command_spec.rb b/services/image_factory/aeolus-image/spec/base_command_spec.rb new file mode 100644 index 0000000..69e0697 --- /dev/null +++ b/services/image_factory/aeolus-image/spec/base_command_spec.rb @@ -0,0 +1,24 @@ +require 'spec_helper'
+module Aeolus
- module Image
- describe BaseCommand do
describe BaseCommand do
it "should determine the correct credentials for HTTP Authentication" do
basec = BaseCommand.new
iwhd = basec.send :iwhd
iwhd.user.should == nil
conductor = basec.send :conductor
conductor.user.should == "admin"
conductor.password.should == "password"
basec = BaseCommand.new({'username' => "testusername", 'password'=> "testpassword"})
conductor = basec.send :conductor
conductor.user.should == "testusername"
conductor.password.should == "testpassword"
end
end
- end
- end
+end \ No newline at end of file diff --git a/services/image_factory/aeolus-image/spec/sample_data/aeolus-cli b/services/image_factory/aeolus-image/spec/sample_data/aeolus-cli new file mode 100644 index 0000000..574a233 --- /dev/null +++ b/services/image_factory/aeolus-image/spec/sample_data/aeolus-cli @@ -0,0 +1,7 @@ +iwhd:
+conductor:
- url: http://localhost:3000/conductor
- username: admin
- password: password
\ No newline at end of file diff --git a/services/image_factory/aeolus-image/spec/spec_helper.rb b/services/image_factory/aeolus-image/spec/spec_helper.rb index d3adf8d..ad0b5cc 100644 --- a/services/image_factory/aeolus-image/spec/spec_helper.rb +++ b/services/image_factory/aeolus-image/spec/spec_helper.rb @@ -32,4 +32,11 @@ end
Spec::Runner.configure do |config| config.include Helpers -end
- config.before(:all) do
- Aeolus::Image::BaseCommand.class_eval do
def load_config
YAML::load(File.open(File.join(File.dirname(__FILE__), "sample_data/aeolus-cli")))
end
- end
- end
+end \ No newline at end of file
From: Martyn Taylor mtaylor@redhat.com
--- .../aeolus-image/lib/config_parser.rb | 10 +++++++++- .../image_factory/aeolus-image/lib/list_command.rb | 5 +++++ .../aeolus-image/spec/list_command_spec.rb | 17 +++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletions(-) create mode 100644 services/image_factory/aeolus-image/spec/list_command_spec.rb
diff --git a/services/image_factory/aeolus-image/lib/config_parser.rb b/services/image_factory/aeolus-image/lib/config_parser.rb index 3ee8b1e..91b637a 100644 --- a/services/image_factory/aeolus-image/lib/config_parser.rb +++ b/services/image_factory/aeolus-image/lib/config_parser.rb @@ -1,3 +1,4 @@ +require 'rubygems' require 'optparse' require 'logger' require 'base_command' @@ -171,7 +172,14 @@ module Aeolus # For the list example above, that object would call a method 'images' based on the item # @options[:subcommand] being :images, so internally that class may do something like: # self.send(@options[:subcommand]) - "Not implemented" + if !@options['subcommand'].nil? + list_command = ListCommand.new(@options) + list_command.send(@options[:subcommand]) + else + # TODO: Pull out Print Usage into seporate method, and print + puts "Could not find subcommand for list, run `./aeolus-image --help` for usage instructions" + exit(1) + end end
def build diff --git a/services/image_factory/aeolus-image/lib/list_command.rb b/services/image_factory/aeolus-image/lib/list_command.rb index 35f6927..9939f74 100644 --- a/services/image_factory/aeolus-image/lib/list_command.rb +++ b/services/image_factory/aeolus-image/lib/list_command.rb @@ -4,6 +4,11 @@ module Aeolus def initialize(opts={}, logger=nil) super(opts, logger) end + + def images + doc = Nokogiri::XML iwhd['/images'].get + doc.xpath("/objects/object/key").collect { |node| "uuid: " + node.text } + end end end end diff --git a/services/image_factory/aeolus-image/spec/list_command_spec.rb b/services/image_factory/aeolus-image/spec/list_command_spec.rb new file mode 100644 index 0000000..01d3646 --- /dev/null +++ b/services/image_factory/aeolus-image/spec/list_command_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper' + +module Aeolus + module Image + describe ListCommand do + it "should return a list of images" do + regexp = Regexp.new('[uuid:\s][\w]{8}[-][\w]{4}[-][\w]{4}[-][\w]{4}[-][\w]{12}') + listc = ListCommand.new + images = listc.images + images.each do |image| + regexp.match(image.to_s).should_not == nil + end + end + + end + end +end \ No newline at end of file
On Tue, 2011-06-14 at 15:55 +0100, mtaylor@redhat.com wrote:
From: Martyn Taylor mtaylor@redhat.com
.../aeolus-image/lib/config_parser.rb | 10 +++++++++- .../image_factory/aeolus-image/lib/list_command.rb | 5 +++++ .../aeolus-image/spec/list_command_spec.rb | 17 +++++++++++++++++ 3 files changed, 31 insertions(+), 1 deletions(-) create mode 100644 services/image_factory/aeolus-image/spec/list_command_spec.rb
ok, this one almost works, but needs a couple minor fixes. First, the tests fail. I think I mentioned this in irc or something, but the test that checks for proper parsing of the command now fails, as the command cannot be executed as is in the test. My suggestion is to change that test to look for the expected failure, and add a new one that actually supplies the needed subcommand (or just calls delete, which is unimplemented, so should not error). The errors I get from 'rake spec' are: 1) 'Aeolus::Image::ListCommand should return a list of images' FAILED expected not: == nil, got: nil ./spec/list_command_spec.rb:11: ./spec/list_command_spec.rb:10:in `each' ./spec/list_command_spec.rb:10:
2) SystemExit in 'Aeolus::Image::ConfigParser should parse the specified command' exit ./spec/config_parser_spec.rb:8:
Finished in 26.533244 seconds
15 examples, 2 failures
Running the command directly with the following works fine after one minor fix: ruby -r 'rubygems' bin/aeolus-image.rb list --images
Couple other fixes listed inline
diff --git a/services/image_factory/aeolus-image/lib/config_parser.rb b/services/image_factory/aeolus-image/lib/config_parser.rb index 3ee8b1e..91b637a 100644 --- a/services/image_factory/aeolus-image/lib/config_parser.rb +++ b/services/image_factory/aeolus-image/lib/config_parser.rb @@ -1,3 +1,4 @@ +require 'rubygems'
rubygems should not be directly required by any of our classes, this will be added by rubygems itself once we package this. In the meantime, everything works as designed by calling: ruby -r 'rubygems' bin/aeolus-image <command> <options>
require 'optparse' require 'logger' require 'base_command' @@ -171,7 +172,14 @@ module Aeolus # For the list example above, that object would call a method 'images' based on the item # @options[:subcommand] being :images, so internally that class may do something like: # self.send(@options[:subcommand])
"Not implemented"
if !@options['subcommand'].nil?
The above check fails, it needs to be looking for @options[:subcommand]. Same comment I made before about switching if to unless here, as I think it is more readable, but that is just a personal preference, so you can leave it if you want.
list_command = ListCommand.new(@options)
list_command.send(@options[:subcommand])
else
# TODO: Pull out Print Usage into seporate method, and print
puts "Could not find subcommand for list, run `./aeolus-image --help` for usage instructions"
exit(1)
end end def build
diff --git a/services/image_factory/aeolus-image/lib/list_command.rb b/services/image_factory/aeolus-image/lib/list_command.rb index 35f6927..9939f74 100644 --- a/services/image_factory/aeolus-image/lib/list_command.rb +++ b/services/image_factory/aeolus-image/lib/list_command.rb @@ -4,6 +4,11 @@ module Aeolus def initialize(opts={}, logger=nil) super(opts, logger) end
def images
doc = Nokogiri::XML iwhd['/images'].get
doc.xpath("/objects/object/key").collect { |node| "uuid: " + node.text }
end endend
end diff --git a/services/image_factory/aeolus-image/spec/list_command_spec.rb b/services/image_factory/aeolus-image/spec/list_command_spec.rb new file mode 100644 index 0000000..01d3646 --- /dev/null +++ b/services/image_factory/aeolus-image/spec/list_command_spec.rb @@ -0,0 +1,17 @@ +require 'spec_helper'
+module Aeolus
- module Image
- describe ListCommand do
it "should return a list of images" do
regexp = Regexp.new('[uuid:\s][\w]{8}[-][\w]{4}[-][\w]{4}[-][\w]{4}[-][\w]{12}')
listc = ListCommand.new
images = listc.images
images.each do |image|
regexp.match(image.to_s).should_not == nil
end
end
- end
- end
+end \ No newline at end of file
aeolus-devel@lists.fedorahosted.org