-
Notifications
You must be signed in to change notification settings - Fork 898
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
use delegate for cpu_total_cores / cpu_cores_per_socket #12911
Conversation
Checked commit kbrock@be31c85 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 |
@@ -157,8 +157,7 @@ class VmOrTemplate < ApplicationRecord | |||
virtual_column :num_hard_disks, :type => :integer, :uses => {:hardware => :hard_disks} | |||
virtual_column :num_disks, :type => :integer, :uses => {:hardware => :disks} | |||
virtual_column :num_cpu, :type => :integer, :uses => :hardware | |||
virtual_column :cpu_total_cores, :type => :integer, :uses => :hardware | |||
virtual_column :cpu_cores_per_socket, :type => :integer, :uses => :hardware | |||
virtual_delegate :cpu_total_cores, :cpu_cores_per_socket, :to => :hardware, :allow_nil => true, :default => 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.
Darga currently doesn't have #10476, which would allow for adding the :default
option that you have here.
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.
thanks - removed from darga
👍 LGTM |
use delegate for cpu_total_cores / cpu_cores_per_socket (cherry picked from commit 91517e8) https://bugzilla.redhat.com/show_bug.cgi?id=1422647
Euwe backport details:
|
The goal is to calculate columns like
aggressive_vcpus_recommended_change_pct
in the database.These columns are calculated from
cpu_total_cores
among others.Using a delegate here will define the arel for us so it can be a virtual attribute with sql.
(The final solution still requires all the other columns be converted to arel as well...)
related to #12733
https://bugzilla.redhat.com/show_bug.cgi?id=1395743
/cc @NickLaMuro FYI