-
Notifications
You must be signed in to change notification settings - Fork 897
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
EmsRefresh task name to use demodularize classname #16594
Conversation
@miq-bot add_labels providers |
1764b81
to
2fccd5b
Compare
describe ".create_refresh_task" do | ||
it "create refresh task and trancates task name to 255 symbols" do | ||
vm = FactoryGirl.create(:vm_vmware, :name => "vm_vmware1", :ext_management_system => @ems) | ||
targets = Array.new(500) { vm } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has been wrong
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch
@miq-bot add_label bug |
app/models/ems_refresh.rb
Outdated
@@ -204,6 +204,9 @@ def self.queue_merge(targets, ems, create_task = false) | |||
end | |||
|
|||
def self.create_refresh_task(ems, targets) | |||
targets = targets.collect do |t| | |||
[t.first.demodulize, t.last] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change this to
targets.collect do |target_class, target_id|
So you don't need to do t.first
/t.last
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Checked commit jameswnl@2d7e450 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 LGTM
https://bugzilla.redhat.com/show_bug.cgi?id=1503660
It used to be like
EmsRefresh(ems_0000000000001) [[["ManageIQ::Providers::Vmware::InfraManager::Vm", 30], ["ManageIQ::Providers::Vmware::InfraManager::Vm", 31]]]
and now it will be
EmsRefresh(ems_0000000000001) [[["Vm", 30], ["Vm", 31]]]