On Fri, Mar 23, 2012 at 09:52:45AM -0400, Tzu-Mainn Chen wrote:
---
src/app/controllers/deployments_controller.rb | 4 +-
src/app/controllers/pools_controller.rb | 12 ++++----
src/app/controllers/users_controller.rb | 4 +-
src/app/models/instance.rb | 9 +++++++
src/app/models/pool.rb | 32 ++++++++++++++++++------
src/app/views/deployments/_list.html.haml | 2 +-
src/app/views/pools/_deployments.html.haml | 2 +-
src/app/views/pools/_pretty_list.html.haml | 8 +++---
8 files changed, 49 insertions(+), 24 deletions(-)
diff --git a/src/app/controllers/deployments_controller.rb
b/src/app/controllers/deployments_controller.rb
index 3eb7aa4..504a050 100644
--- a/src/app/controllers/deployments_controller.rb
+++ b/src/app/controllers/deployments_controller.rb
@@ -383,7 +383,7 @@ class DeploymentsController < ApplicationController
{ :name => t("providers.provider"), :sortable => false }
]
@pools = Pool.list_for_user(current_user, Privilege::CREATE, Deployment)
- @deployments = Deployment.includes(:owner).
+ @deployments = Deployment.includes(:owner, :pool, :instances).
apply_filters(:preset_filter_id =>
params[:deployments_preset_filter], :search_filter => params[:deployments_search]).
list_for_user(current_user, Privilege::VIEW).
where('deployments.pool_id' => @pools).
@@ -410,7 +410,7 @@ class DeploymentsController < ApplicationController
end
def init_new_deployment_attrs
- @deployables = Deployable.list_for_user(current_user, Privilege::USE).select{|d|
d.catalogs.collect {|c| c.pool}.include?(@pool)}
+ @deployables = Deployable.includes({:catalogs =>
:pool}).list_for_user(current_user, Privilege::USE).select{|d| d.catalogs.collect{|c|
c.pool}.include?(@pool)}
@pools = Pool.list_for_user(current_user, Privilege::CREATE, Deployment)
@deployable = params[:deployable_id] ? Deployable.find(params[:deployable_id]) :
nil
@realms = FrontendRealm.all
diff --git a/src/app/controllers/pools_controller.rb
b/src/app/controllers/pools_controller.rb
index cfefe34..abc3203 100644
--- a/src/app/controllers/pools_controller.rb
+++ b/src/app/controllers/pools_controller.rb
@@ -47,21 +47,21 @@ class PoolsController < ApplicationController
if filter_view?
case @details_tab[:id]
when 'pools'
- @pools = Pool.apply_filters(:preset_filter_id =>
params[:pools_preset_filter], :search_filter => params[:pools_search]).
+ @pools = Pool.includes(:deployments, :instances).apply_filters(:preset_filter_id
=> params[:pools_preset_filter], :search_filter => params[:pools_search]).
list_for_user(current_user,
Privilege::VIEW).list(sort_column(Pool), sort_direction).
paginate(:page => params[:page], :per_page => PER_PAGE)
when 'instances'
params[:instances_preset_filter] = "" unless
params[:instances_preset_filter]
- @instances = Instance.apply_filters(:preset_filter_id =>
params[:instances_preset_filter], :search_filter => params[:instances_search]).
+ @instances = Instance.includes({:provider_account =>
:provider}).apply_filters(:preset_filter_id => params[:instances_preset_filter],
:search_filter => params[:instances_search]).
list_for_user(current_user,
Privilege::VIEW).list(sort_column(Instance), sort_direction).
paginate(:page => params[:page], :per_page =>
PER_PAGE)
when 'deployments'
- @deployments = Deployment.apply_filters(:preset_filter_id =>
params[:deployments_preset_filter], :search_filter => params[:deployments_search]).
+ @deployments = Deployment.includes(:pool,
:instances).apply_filters(:preset_filter_id => params[:deployments_preset_filter],
:search_filter => params[:deployments_search]).
list_for_user(current_user,
Privilege::VIEW).list(sort_column(Deployment), sort_direction).
paginate(:page => params[:page], :per_page =>
PER_PAGE)
end
else
- @pools = Pool.list_for_user(current_user, Privilege::VIEW).list(sort_column(Pool),
sort_direction).
+ @pools = Pool.includes(:deployments, :instances, :quota,
:catalogs).list_for_user(current_user, Privilege::VIEW).list(sort_column(Pool),
sort_direction).
paginate(:page => params[:page], :per_page => PER_PAGE)
end
@@ -109,7 +109,7 @@ class PoolsController < ApplicationController
details_tab_name = params[:details_tab].blank? ? 'deployments' :
params[:details_tab]
@details_tab = @tabs.find {|t| t[:id] == details_tab_name} ||
@tabs.first[:name].downcase
- @deployments = @pool.deployments.
+ @deployments = @pool.deployments.includes(:owner, :pool, :instances, :events).
apply_filters(:preset_filter_id =>
params[:deployments_preset_filter], :search_filter => params[:deployments_search]).
list_for_user(current_user, Privilege::VIEW).
paginate(:page => params[:page], :per_page => PER_PAGE)
if @details_tab[:id] == 'deployments'
@@ -326,7 +326,7 @@ class PoolsController < ApplicationController
instances = current_user.owned_instances
failed_instances = instances.failed
@statistics = {
- :pools_in_use => @user_pools.select { |pool|
pool.instances.pending.count > 0 || pool.instances.deployed.count > 0 }.count,
+ :pools_in_use => @user_pools.select { |pool|
pool.instances.pending_or_deployed.count > 0 }.count,
:deployments => current_user.deployments.count,
:instances => instances.count,
:instances_pending => instances.select {|instance| instance.state ==
Instance::STATE_NEW || instance.state == Instance::STATE_PENDING}.count,
diff --git a/src/app/controllers/users_controller.rb
b/src/app/controllers/users_controller.rb
index 93918bc..010ebe1 100644
--- a/src/app/controllers/users_controller.rb
+++ b/src/app/controllers/users_controller.rb
@@ -157,7 +157,7 @@ class UsersController < ApplicationController
protected
def load_users
- @users = User.apply_filters(:preset_filter_id => params[:users_preset_filter],
:search_filter => params[:users_search])
+ @users = User.includes(:quota).apply_filters(:preset_filter_id =>
params[:users_preset_filter], :search_filter => params[:users_search])
sort_order = params[:sort_by].nil? ? "login" : params[:sort_by]
# TODO: (lmartinc) Optimize this sort! hell!
if sort_order == "percentage_quota_used"
@@ -165,7 +165,7 @@ class UsersController < ApplicationController
elsif sort_order == "quota"
@users.sort! {|x,y| (x.quota.maximum_running_instances and
y.quota.maximum_running_instances) ? x.quota.maximum_running_instances <=>
y.quota.maximum_running_instances : (x ? 1 : -1) }
else
- @users = User.apply_filters(:preset_filter_id => params[:users_preset_filter],
:search_filter => params[:users_search]).order(sort_order)
+ @users = User.includes(:quota).apply_filters(:preset_filter_id =>
params[:users_preset_filter], :search_filter =>
params[:users_search]).order(sort_order)
end
end
diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb
index 855045a..60a0f09 100644
--- a/src/app/models/instance.rb
+++ b/src/app/models/instance.rb
@@ -120,6 +120,7 @@ class Instance < ActiveRecord::Base
scope :deployed, :conditions => { :state => [STATE_RUNNING,
STATE_SHUTTING_DOWN] }
# FIXME: "pending" is misleading as it doesn't just cover STATE_PENDING
scope :pending, :conditions => { :state => [STATE_NEW, STATE_PENDING] }
+ scope :pending_or_deployed, :conditions => { :state => [STATE_NEW,
STATE_PENDING, STATE_RUNNING, STATE_SHUTTING_DOWN] }
# FIXME: "failed" is misleading too...
scope :failed, :conditions => { :state => FAILED_STATES }
scope :stopped, :conditions => {:state => STATE_STOPPED}
@@ -478,6 +479,14 @@ class Instance < ActiveRecord::Base
[STATE_RUNNING, STATE_SHUTTING_DOWN].include?(state)
end
+ def stopped?
+ [STATE_STOPPED].include?(state)
+ end
+
+ def pending?
+ [STATE_NEW, STATE_PENDING].include?(state)
+ end
+
def uptime
deployed? ? (Time.now - time_last_running) : 0
end
diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
index 9d1af3a..57a6caf 100644
--- a/src/app/models/pool.rb
+++ b/src/app/models/pool.rb
@@ -78,6 +78,22 @@ class Pool < ActiveRecord::Base
instances.all? {|i| i.destroyable? }
end
+ def failed_instances
+ instances.select {|instance| instance.failed?}
+ end
+
+ def not_stopped_instances
+ instances.select {|instance| !instance.stopped?}
+ end
+
+ def deployed_instances
+ instances.select {|instance| instance.deployed?}
+ end
+
+ def pending_instances
+ instances.select {|instance| instance.pending?}
+ end
Just for the record, we discussed the above few lines on IRC a bit. I
had a knee-jerk reaction against this, since it's replacing existing
named scopes with pulling back _all_ instances and sorting them in
memory. But it turns out that the use case is that we should already
have "instances" in cache, so it's quicker to just sort over them,
rather than firing off new queries using each scope here.
+
# TODO: Implement Alerts and Updates
def statistics(user = nil)
# TODO - Need to set up cache invalidation before this is safe
@@ -85,17 +101,17 @@ class Pool < ActiveRecord::Base
max = quota.maximum_running_instances
total = quota.running_instances
avail = max - total unless max.nil?
- failed = (user.nil? ? instances.failed :
- instances.failed.list_for_user(user, Privilege::VIEW))
+ failed = (user.nil? || failed_instances.empty? ? failed_instances :
+ failed_instances.list_for_user(user, Privilege::VIEW))
statistics = {
- :cloud_providers => instances.collect{|i| i.provider_account}.uniq.count,
- :deployments => deployments.count,
- :total_instances => instances.not_stopped.count,
- :instances_deployed => instances.deployed.count,
- :instances_pending => instances.pending.count,
+ :cloud_providers => instances.includes(:provider_account).collect{|i|
i.provider_account}.uniq.count,
+ :deployments => deployments.size,
+ :total_instances => not_stopped_instances.count,
+ :instances_deployed => deployed_instances.count,
+ :instances_pending => pending_instances.count,
:instances_failed => failed,
:instances_failed_visible_count => failed.count,
- :instances_failed_count => instances.failed.count,
+ :instances_failed_count => failed_instances.count,
:used_quota => quota.running_instances,
:quota_percent => number_to_percentage(quota.percentage_used,
:precision => 0),
diff --git a/src/app/views/deployments/_list.html.haml
b/src/app/views/deployments/_list.html.haml
index b123219..d593cc6 100644
--- a/src/app/views/deployments/_list.html.haml
+++ b/src/app/views/deployments/_list.html.haml
@@ -36,7 +36,7 @@
%td= deployment.created_at
%td= deployment.deployable_xml.name
%td= deployment.status
- %td{:class => 'center'}= deployment.instances.count
+ %td{:class => 'center'}= deployment.instances.size
Another "just for the record" from IRC discussion: apparently
".count"
references "SELECT COUNT" on the association even if we had already
looked it up, while ".size" is more flexible -- array size if we have
it, otherwise, "SELECT COUNT".
%td= link_to deployment.pool.name, pool_path(deployment.pool,
:view => :filter)
%td= owner_name(deployment)
%td= link_to(deployment.provider.provider_type.name, deployment.provider) if
deployment.provider
diff --git a/src/app/views/pools/_deployments.html.haml
b/src/app/views/pools/_deployments.html.haml
index b4aed1d..d91bff5 100644
--- a/src/app/views/pools/_deployments.html.haml
+++ b/src/app/views/pools/_deployments.html.haml
@@ -11,7 +11,7 @@
%ul
%li.right
%dt.instances.count= t 'instances.instances.other'
- %dd= deployment.instances.count || 0
+ %dd= deployment.instances.size || 0
%li.left
%dt=t :uptime
diff --git a/src/app/views/pools/_pretty_list.html.haml
b/src/app/views/pools/_pretty_list.html.haml
index 541f85a..47e7a8b 100644
--- a/src/app/views/pools/_pretty_list.html.haml
+++ b/src/app/views/pools/_pretty_list.html.haml
@@ -4,7 +4,7 @@
.content#pools-list
- @pools.each do |pool|
- %div.pool.overview{:id => ['pool', pool.id], :class =>
('collapsed' unless pool.deployments.count > 0)}
+ %div.pool.overview{:id => ['pool', pool.id], :class =>
('collapsed' unless pool.deployments.size > 0)}
%header
%h3.name
= link_to pool.name, pool
@@ -12,10 +12,10 @@
%ul.groups
%li
%dt=t 'deployments.deployments'
- %dd.count.deployment= pool.deployments.count
+ %dd.count.deployment= pool.deployments.size
%li
%dt=t 'instances.instances.other'
- %dd.count.instance= pool.instances.count
+ %dd.count.instance= pool.instances.size
%dd.count.instance.pending= pool.instances.select {|i| i.state ==
Instance::STATE_PENDING }.length
%dd.count.instance.failure= pool.instances.select {|i| i.state ==
Instance::STATE_ERROR || i.state == Instance::STATE_CREATE_FAILED }.length
%li
@@ -25,7 +25,7 @@
%a.control{:href => '#'}
%span Expand/Collapse
%div.content
- - deployments = pool.deployments.list_for_user(current_user,
Privilege::VIEW).ascending_by_name.paginate(:page => params[:page], :per_page =>
PER_PAGE)
+ - deployments = pool.deployments.includes(:owner, :pool, :instances,
:events).list_for_user(current_user, Privilege::VIEW).ascending_by_name.paginate(:page
=> params[:page], :per_page => PER_PAGE)
= render :partial => 'deployments', :locals => {:pool => pool,
:deployments => deployments}
%ul.content.actions
%li
--
1.7.6.5
ACK. I put a couple of inline comments to reflect our discussion on IRC.
-- Matt