From 508683b937b4319f9d07c863b9a9a562625c01dc Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Thu, 5 Sep 2019 11:46:18 -0400 Subject: [PATCH 1/2] Don't require Job.create_job to pass in the class Instead of passing in the class name as the first argument to Job.create_job you can just call .create_job on the end class --- app/models/job.rb | 5 ++--- app/models/vm_or_template/scanning.rb | 2 +- spec/models/job_proxy_dispatcher_spec.rb | 14 +++++++------- spec/models/job_spec.rb | 12 ++++++------ spec/models/miq_task/purging_spec.rb | 4 ++-- spec/models/miq_task_spec.rb | 8 ++++---- 6 files changed, 22 insertions(+), 23 deletions(-) diff --git a/app/models/job.rb b/app/models/job.rb index 47d49afd641..c568c8fc003 100644 --- a/app/models/job.rb +++ b/app/models/job.rb @@ -16,10 +16,9 @@ class Job < ApplicationRecord DEFAULT_TIMEOUT = 300 DEFAULT_USERID = 'system'.freeze - def self.create_job(process_type, options = {}) - klass = Object.const_get(process_type) + def self.create_job(options = {}) ar_options = options.dup.delete_if { |k, _v| !Job.column_names.include?(k.to_s) } - job = klass.new(ar_options) + job = new(ar_options) job.options = options job.initialize_attributes job.save diff --git a/app/models/vm_or_template/scanning.rb b/app/models/vm_or_template/scanning.rb index 081f5a73a0e..d0d0e106268 100644 --- a/app/models/vm_or_template/scanning.rb +++ b/app/models/vm_or_template/scanning.rb @@ -29,7 +29,7 @@ def raw_scan(userid = "system", options = {}) self.last_scan_attempt_on = Time.now.utc save - job = Job.create_job("VmScan", options) + job = VmScan.create_job(options) return job rescue => err _log.log_backtrace(err) diff --git a/spec/models/job_proxy_dispatcher_spec.rb b/spec/models/job_proxy_dispatcher_spec.rb index 3f1bca7e4b5..2e641311ef7 100644 --- a/spec/models/job_proxy_dispatcher_spec.rb +++ b/spec/models/job_proxy_dispatcher_spec.rb @@ -265,11 +265,11 @@ end context "limiting number of smart state analysis running on one server" do - let(:job) { Job.create_job("VmScan", :miq_server_id => @server.id, :name => "Hello - 1") } + let(:job) { VmScan.create_job(:miq_server_id => @server.id, :name => "Hello - 1") } before do - Job.create_job("VmScan", :miq_server_id => @server.id, :name => "Hello - 2") + VmScan.create_job(:miq_server_id => @server.id, :name => "Hello - 2") .update_attributes(:dispatch_status => "active") - Job.create_job("VmScan", :miq_server_id => @server.id, :name => "Hello - 3") + VmScan.create_job(:miq_server_id => @server.id, :name => "Hello - 3") .update_attributes(:dispatch_status => "active") end @@ -302,7 +302,7 @@ describe "#start_job_on_proxy" do it "creates job options and passing it to `queue_signal'" do - job = Job.create_job("VmScan", :miq_server_id => @server.id, :name => "Hello, World") + job = VmScan.create_job(:miq_server_id => @server.id, :name => "Hello, World") dispatcher.instance_variable_set(:@active_vm_scans_by_zone, @server.my_zone => 0) job_options = {:args => ["start"], :zone => @server.my_zone, :server_guid => @server.guid, :role => "smartproxy"} @@ -315,7 +315,7 @@ describe "#do_dispatch" do let(:ems_id) { 1 } - let(:job) { Job.create_job("VmScan", :name => "Hello, World") } + let(:job) { VmScan.create_job(:name => "Hello, World") } before do dispatcher.instance_variable_set(:@active_container_scans_by_zone_and_ems, @server.my_zone => {ems_id => 0}) @@ -358,7 +358,7 @@ end describe "#queue_signal" do - let(:job) { Job.create_job("VmScan", :name => "Hello, World") } + let(:job) { VmScan.create_job(:name => "Hello, World") } it "queues call to Job::StateMachine#signal_abort if signal is 'abort'" do options = {:args => [:abort]} @@ -385,7 +385,7 @@ describe "#dispatch_to_ems" do let(:ems_id) { 1 } let(:jobs) do - [Job.create_job("VmScan", :name => "Hello, World 1"), Job.create_job("VmScan", :name => "Hello, World 2")] + [VmScan.create_job(:name => "Hello, World 1"), VmScan.create_job(:name => "Hello, World 2")] end it "dispatches all supplied jobs if supplied concurency limit is 0" do diff --git a/spec/models/job_spec.rb b/spec/models/job_spec.rb index 0a297883966..111cb9b5ba6 100644 --- a/spec/models/job_spec.rb +++ b/spec/models/job_spec.rb @@ -273,7 +273,7 @@ end it "returns the correct adjusment 1 if target class was not defined" do - job_without_target = Job.create_job("VmScan", :target_class => nil) + job_without_target = VmScan.create_job(:target_class => nil) expect(job_without_target.timeout_adjustment).to eq(1) end end @@ -329,7 +329,7 @@ context "before_destroy callback" do before do - @job = Job.create_job("VmScan", :name => "Hello, World!") + @job = VmScan.create_job(:name => "Hello, World!") end it "allows to delete not active job" do @@ -348,7 +348,7 @@ describe "#attributes_log" do it "returns attributes for logging" do - job = Job.create_job("VmScan", :name => "Hello, World!") + job = VmScan.create_job(:name => "Hello, World!") expect(job.attributes_log).to include("VmScan", "Hello, World!", job.guid) end end @@ -356,7 +356,7 @@ context "belongs_to task" do let(:job_name) { "Hello, World!" } before do - @job = Job.create_job("VmScan", :name => job_name) + @job = VmScan.create_job(:name => job_name) @task = MiqTask.find_by(:name => job_name) end @@ -428,7 +428,7 @@ let(:message) { "Very Interesting Message" } it "updates 'jobs.message' column" do - Job.create_job("VmScan").update_message(message) + VmScan.create_job.update_message(message) expect(Job.first.message).to eq message end end @@ -438,7 +438,7 @@ let(:guid) { "qwerty" } it "finds job by passed guid and updates 'message' column for found record" do - Job.create_job("VmScan", :guid => guid) + VmScan.create_job(:guid => guid) Job.update_message(guid, message) expect(Job.find_by(:guid => guid).message).to eq message end diff --git a/spec/models/miq_task/purging_spec.rb b/spec/models/miq_task/purging_spec.rb index 0114a64846e..a43a2da6a4d 100644 --- a/spec/models/miq_task/purging_spec.rb +++ b/spec/models/miq_task/purging_spec.rb @@ -3,13 +3,13 @@ describe ".purge_by_date" do before do Timecop.freeze(8.days.ago) do - @old_task = Job.create_job("VmScan", :guid => "old").miq_task + @old_task = VmScan.create_job(:guid => "old").miq_task FactoryBot.create(:binary_blob, :name => "old", :resource_type => 'MiqTask', :resource_id => @old_task.id) FactoryBot.create(:log_file, :name => "old", :miq_task_id => @old_task.id) end Timecop.freeze(6.days.ago) do - @new_task = Job.create_job("VmScan", :guid => "recent").miq_task + @new_task = VmScan.create_job(:guid => "recent").miq_task @new_task.state_finished FactoryBot.create(:binary_blob, :name => "recent", :resource_type => 'MiqTask', :resource_id => @new_task.id) FactoryBot.create(:log_file, :name => "recent", :miq_task_id => @new_task.id) diff --git a/spec/models/miq_task_spec.rb b/spec/models/miq_task_spec.rb index 2edfa328e65..b5b2edf17ea 100644 --- a/spec/models/miq_task_spec.rb +++ b/spec/models/miq_task_spec.rb @@ -312,7 +312,7 @@ it "doesn't destroy miq_task and associated job if job is active" do expect(MiqTask.count).to eq 0 - job = Job.create_job("VmScan") + job = VmScan.create_job job.update_attributes!(:state => "active") expect(MiqTask.count).to eq 1 MiqTask.first.destroy @@ -331,7 +331,7 @@ it "destroys miq_task record and job record if job associated with it 'finished'" do expect(MiqTask.count).to eq 0 - job = Job.create_job("VmScan") + job = VmScan.create_job job.update_attributes!(:state => "finished") expect(MiqTask.count).to eq 1 MiqTask.first.destroy @@ -341,7 +341,7 @@ it "destroys miq_task record and job record if job associated with it not started yet" do expect(MiqTask.count).to eq 0 - job = Job.create_job("VmScan") + job = VmScan.create_job job.update_attributes!(:state => "waiting_to_start") expect(MiqTask.count).to eq 1 MiqTask.first.destroy @@ -472,7 +472,7 @@ context "task linked to job" do let(:miq_task) do - job = Job.create_job("VmScan") + job = VmScan.create_job job.miq_task end From f97e6d151ba59a306e1882734e58db9d3f4732ef Mon Sep 17 00:00:00 2001 From: Adam Grare Date: Fri, 6 Sep 2019 08:32:17 -0400 Subject: [PATCH 2/2] Remove create_job where it was just passing name --- app/models/infra_conversion_job.rb | 4 ---- app/models/manageiq/providers/ansible_runner_workflow.rb | 2 +- .../providers/cloud_manager/orchestration_template_runner.rb | 5 ----- .../embedded_ansible/automation_manager/playbook_runner.rb | 2 +- app/models/manageiq/providers/ems_refresh_workflow.rb | 4 ---- 5 files changed, 2 insertions(+), 15 deletions(-) diff --git a/app/models/infra_conversion_job.rb b/app/models/infra_conversion_job.rb index 7ebdf0303b1..84b9b3852fd 100644 --- a/app/models/infra_conversion_job.rb +++ b/app/models/infra_conversion_job.rb @@ -1,8 +1,4 @@ class InfraConversionJob < Job - def self.create_job(options) - super(name, options) - end - # # State-transition diagram: # :poll_conversion :poll_post_stage diff --git a/app/models/manageiq/providers/ansible_runner_workflow.rb b/app/models/manageiq/providers/ansible_runner_workflow.rb index f420a187191..f60747cd7bb 100644 --- a/app/models/manageiq/providers/ansible_runner_workflow.rb +++ b/app/models/manageiq/providers/ansible_runner_workflow.rb @@ -2,7 +2,7 @@ class ManageIQ::Providers::AnsibleRunnerWorkflow < Job def self.create_job(env_vars, extra_vars, role_or_playbook_options, hosts = ["localhost"], credentials = [], timeout: 1.hour, poll_interval: 1.second, verbosity: 0, become_enabled: false) - super(name, role_or_playbook_options.merge( + super(role_or_playbook_options.merge( :become_enabled => become_enabled, :credentials => credentials, :env_vars => env_vars, diff --git a/app/models/manageiq/providers/cloud_manager/orchestration_template_runner.rb b/app/models/manageiq/providers/cloud_manager/orchestration_template_runner.rb index 013790ca30c..3cb0978992d 100644 --- a/app/models/manageiq/providers/cloud_manager/orchestration_template_runner.rb +++ b/app/models/manageiq/providers/cloud_manager/orchestration_template_runner.rb @@ -1,11 +1,6 @@ class ManageIQ::Providers::CloudManager::OrchestrationTemplateRunner < ::Job DEFAULT_EXECUTION_TTL = 10 # minutes - # options are job table columns, including options column which is the playbook context info - def self.create_job(options) - super(name, options) - end - def minimize_indirect @minimize_indirect = true if @minimize_indirect.nil? @minimize_indirect diff --git a/app/models/manageiq/providers/embedded_ansible/automation_manager/playbook_runner.rb b/app/models/manageiq/providers/embedded_ansible/automation_manager/playbook_runner.rb index 634d39c9478..3018cfb1466 100644 --- a/app/models/manageiq/providers/embedded_ansible/automation_manager/playbook_runner.rb +++ b/app/models/manageiq/providers/embedded_ansible/automation_manager/playbook_runner.rb @@ -3,7 +3,7 @@ class ManageIQ::Providers::EmbeddedAnsible::AutomationManager::PlaybookRunner < # options are job table columns, including options column which is the playbook context info def self.create_job(options) - super(name, options.with_indifferent_access) + super(options.with_indifferent_access) end def minimize_indirect diff --git a/app/models/manageiq/providers/ems_refresh_workflow.rb b/app/models/manageiq/providers/ems_refresh_workflow.rb index 57c172a24f6..22e509d9deb 100644 --- a/app/models/manageiq/providers/ems_refresh_workflow.rb +++ b/app/models/manageiq/providers/ems_refresh_workflow.rb @@ -1,8 +1,4 @@ class ManageIQ::Providers::EmsRefreshWorkflow < Job - def self.create_job(options) - super(name, options) - end - # # State-transition diagram: # :poll_native_task