On Mon, 2011-03-14 at 21:54 -0700, Richard Su wrote:
The jobs submits requests to the console. The console communicates
with the new factory. The responses are in xml and indicates the uuid.
These comments are just from reading, as I am in the middle of a task,
so I can't apply this yet. I asked Scott to test it out, as I think he
can get to it before me. Note that we probably can't push it though
until the rest of the config bit are in. Overall looks good, just a few
bit of feedback for now.
---
src/app/models/build_job.rb | 69 ++++++++++++++++++++++++++++++++
src/app/models/image.rb | 17 ++------
src/app/models/provider_image.rb | 6 ---
src/app/models/push_job.rb | 72 ++++++++++++++++++++++++++++++++++
src/config/image_factory_console.yml | 2 +
src/spec/factories/build_job.rb | 7 +++
src/spec/factories/push_job.rb | 7 +++
src/spec/models/build_job_spec.rb | 10 +++++
src/spec/models/push_job_spec.rb | 11 +++++
9 files changed, 182 insertions(+), 19 deletions(-)
create mode 100644 src/app/models/build_job.rb
create mode 100644 src/app/models/push_job.rb
create mode 100644 src/config/image_factory_console.yml
create mode 100644 src/spec/factories/build_job.rb
create mode 100644 src/spec/factories/push_job.rb
create mode 100644 src/spec/models/build_job_spec.rb
create mode 100644 src/spec/models/push_job_spec.rb
diff --git a/src/app/models/build_job.rb b/src/app/models/build_job.rb
new file mode 100644
index 0000000..08450da
--- /dev/null
+++ b/src/app/models/build_job.rb
@@ -0,0 +1,69 @@
+#
+# Copyright (C) 2011 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA 02110-1301, USA. A copy of the GNU General Public License is
+# also available at
http://www.gnu.org/copyleft/gpl.html.
+
+require 'typhoeus'
+require 'nokogiri'
+
+class BuildJob < Struct.new(:image_id, :hydra)
+
+ def perform
+ @hydra = hydra
+ print "--- start ---\n"
+ print "BuildJob for image_id: "
+ print image_id
+ print "\n"
This should be using a logger instead of print (comment applies to all
instances of print). Note that the logger (and puts) do not need
newlines in the output, each call to either of those methods will be on
a new line.
+ return_uuid = nil
This line ^ should not be needed, as you set the value below. It will
automatically be nil if you have no text to set it with.
+ begin
+ if (@hydra == nil)
+ @hydra = Typhoeus::Hydra.new
+ end
+ image = Image.find(image_id)
+ template_url =
YAML.load_file("#{RAILS_ROOT}/config/image_warehouse.yml")['baseurl'] +
"/templates/" + image.template.uuid
I would put this url-building ^ in the template model, as a helper
method
+ request =
Typhoeus::Request.new(YAML.load_file("#{RAILS_ROOT}/config/image_factory_console.yml")['buildurl'],
+ :method => :post,
+ :timeout => 60*1000, # in milliseconds
+ :params => {
+ :template => template_url,
+ :target => image.provider_type.codename})
+ request.on_complete do |response|
+ if response.success?
+ print "success "
+ xml = Nokogiri::XML(response.body)
+ return_uuid = xml.xpath('//image/uuid').first.text
+ image.uuid = return_uuid
+ image.save
+ print "uuid: "
+ print return_uuid
+ print "\n"
+ else
+ print "failure\n"
+ print response.body
+ print "\n"
+ end
+ end
+ @hydra.queue(request)
+ @hydra.run
+ rescue Exception => e
+ print "Exception: \n"
+ print e
+ print "\n"
+ end
+ print "--- done --- \n"
+ return return_uuid
+ end
+end
diff --git a/src/app/models/image.rb b/src/app/models/image.rb
index fc33b53..2bb8ad9 100644
--- a/src/app/models/image.rb
+++ b/src/app/models/image.rb
@@ -91,15 +91,6 @@ class Image < ActiveRecord::Base
end
end
- def build
- # TODO: this is just stubbed upload call,
- # when new image_builder_service is done, replace
- # with real request to image_builder_service
- unless self.provider_type.build_supported
- raise "Build is not supported on images with provider type
#{self.provider_type.name}"
- end
- end
-
def self.create_and_build!(template, provider_type)
if Image.find_by_template_id_and_provider_type_id(template.id, provider_type.id)
raise ImageExistsError, "An attempted build of this template for the target
'#{provider_type.name}' already exists"
@@ -114,10 +105,9 @@ class Image < ActiveRecord::Base
:template_id => template.id,
:status => Image::STATE_QUEUED
)
- img.delay.build
+ Delayed::Job.enqueue(BuildJob.new(img.id))
img
end
-
def self.single_import(providername, username, password, image_id)
account = Image.get_account(providername, username, password)
Image.import(account, image_id)
@@ -186,11 +176,12 @@ class Image < ActiveRecord::Base
provider_type.providers.each do |p|
# upload only to providers with account
unless p.provider_accounts.empty?
- ProviderImage.create!(
+ pimg = ProviderImage.create!(
:uuid => UUIDTools::UUID.timestamp_create.to_s,
This is not your code and perhaps should be fixed in a later patch, but
we should not be setting a uuid for either image or provider image on
creation, as factory does this. Since you overwrite it with the
response, it is probably ok for now (maybe even less invasive), but this
will need to be fixed soon.
:image => self,
:provider => p
- ).delay.push
+ )
+ Delayed::Job.enqueue(PushJob.new(pimg.id))
end
end
end
diff --git a/src/app/models/provider_image.rb b/src/app/models/provider_image.rb
index e218970..5e5ed3f 100644
--- a/src/app/models/provider_image.rb
+++ b/src/app/models/provider_image.rb
@@ -24,12 +24,6 @@ class ProviderImage < ActiveRecord::Base
validates_uniqueness_of :uuid, :allow_nil => true
validates_uniqueness_of :image_id, :scope => :provider_id
- def push
- # TODO: this is just stubbed upload call,
- # when new image_builder_service is done, replace
- # with real request to image_builder_service
- end
-
def after_update
if self.uploaded_changed? and self.uploaded == true
begin
diff --git a/src/app/models/push_job.rb b/src/app/models/push_job.rb
new file mode 100644
index 0000000..deec592
--- /dev/null
+++ b/src/app/models/push_job.rb
@@ -0,0 +1,72 @@
+#
+# Copyright (C) 2011 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
+# MA 02110-1301, USA. A copy of the GNU General Public License is
+# also available at
http://www.gnu.org/copyleft/gpl.html.
+
+require 'typhoeus'
+require 'nokogiri'
+
+class PushJob < Struct.new(:provider_image_id, :hydra)
+ def perform
+ print "--- start ---\n"
+ print "PushJob for provider_image_id: "
+ print provider_image_id
+ print "\n"
+ # TODO: what if a provider has multiple accounts
+ # for now pick first account
+ begin
+ return_uuid = nil
+ @hydra = hydra
+ if (@hydra == nil)
+ @hydra = Typhoeus::Hydra.new
+ end
+ provider_image = ProviderImage.find(provider_image_id)
+ provider_account = provider_image.provider.provider_accounts.first
+ request =
Typhoeus::Request.new(YAML.load_file("#{RAILS_ROOT}/config/image_factory_console.yml")['pushurl'],
+ :method => :post,
+ :timeout => 60*1000, # in milliseconds
+ :params => {
+ :image_id => provider_image.image.uuid,
+ :provider => provider_image.provider.name,
+ :credentials =>
provider_account.build_credentials
+ })
+ request.on_complete do |response|
+ if response.success?
+ print "success "
+ xml = Nokogiri::XML(response.body)
+ return_uuid = xml.xpath('//image/uuid').first.text
+ provider_image.uuid = return_uuid
+ provider_image.uploaded = true
+ provider_image.save
+ print "uuid: "
+ print return_uuid
+ print "\n"
+ else
+ print "failure\n"
+ print response.body
+ end
+ end
+ @hydra.queue(request)
+ @hydra.run
+ rescue Exception => e
+ print "Exception: \n"
+ print e
+ print "\n"
+ end
+ print "--- done --- \n"
+ return return_uuid
+ end
+end
diff --git a/src/config/image_factory_console.yml b/src/config/image_factory_console.yml
new file mode 100644
index 0000000..b5d66d6
--- /dev/null
+++ b/src/config/image_factory_console.yml
@@ -0,0 +1,2 @@
+buildurl:
http://localhost:2003/build
+pushurl:
http://localhost:2003/push
diff --git a/src/spec/factories/build_job.rb b/src/spec/factories/build_job.rb
new file mode 100644
index 0000000..6f3186c
--- /dev/null
+++ b/src/spec/factories/build_job.rb
@@ -0,0 +1,7 @@
+Factory.define :build_job do |p|
+ hydra = Typhoeus::Hydra.new
+ response = Typhoeus::Response.new(:code => 200, :headers => "", :body
=> "<?xml version=\"1.0\"
encoding=\"UTF-8\"><image><uuid>build-job-stub-id</uuid></image>",
:time => 0.1)
+ hydra.stub(:post,
YAML.load_file("#{RAILS_ROOT}/config/image_factory_console.yml")['buildurl']).and_return(response)
+ p.image_id 1
We should not set hard IDs in factories, they should be automatic, and
they are checked against <factory-object>.id
+ p.hydra hydra
+end
diff --git a/src/spec/factories/push_job.rb b/src/spec/factories/push_job.rb
new file mode 100644
index 0000000..f4346dc
--- /dev/null
+++ b/src/spec/factories/push_job.rb
@@ -0,0 +1,7 @@
+Factory.define :push_job do |p|
+ hydra = Typhoeus::Hydra.new
+ response = Typhoeus::Response.new(:code => 200, :headers => "", :body
=> "<?xml version=\"1.0\"
encoding=\"UTF-8\"><image><uuid>push-job-stub-id</uuid></image>",
:time => 0.1)
+ hydra.stub(:post,
YAML.load_file("#{RAILS_ROOT}/config/image_factory_console.yml")['pushurl']).and_return(response)
+ p.provider_image_id 10
Same comment on IDs
+ p.hydra hydra
+end
diff --git a/src/spec/models/build_job_spec.rb b/src/spec/models/build_job_spec.rb
new file mode 100644
index 0000000..aa4cae7
--- /dev/null
+++ b/src/spec/models/build_job_spec.rb
@@ -0,0 +1,10 @@
+require 'spec_helper'
+
+describe BuildJob do
+ it "should match request and parse stubbed xml" do
+ image = Factory.create(:image, :id => 1)
+ job = Factory.build(:build_job)
+ uuid = job.perform
+ uuid.should == "build-job-stub-id"
+ end
+end
diff --git a/src/spec/models/push_job_spec.rb b/src/spec/models/push_job_spec.rb
new file mode 100644
index 0000000..b728478
--- /dev/null
+++ b/src/spec/models/push_job_spec.rb
@@ -0,0 +1,11 @@
+require 'spec_helper'
+
+describe PushJob do
+ it "should match request and parse stubbed xml" do
+ provider_account = Factory.create(:mock_provider_account)
+ provider_image = Factory.create(:provider_image, :id => 10, :provider =>
provider_account.provider)
You should be able to just specify the provider account, and can even do
it inline like this:
provider_image = Factory.create(:provider_image, :id => 10, :provider =>
Factory.create(:mock_provider_account))
though it may require a little rework of your factories, mainly to point
them at the right providerImage. Actually, now that I think about it,
that whole reference chain should probably just be in the push_job
factory.
+ job = Factory.build(:push_job)
+ uuid = job.perform
+ uuid.should == "push-job-stub-id"
+ end
+end