[PATCH 4/4] BZ 802571 refactor provider_account sort-by-priority to correctly sort nils, regardless of db
Matt Wagner
matt.wagner at redhat.com
Mon Mar 26 16:00:56 UTC 2012
On Fri, Mar 23, 2012 at 09:52:48AM -0400, Tzu-Mainn Chen wrote:
> ---
> src/app/models/deployment.rb | 3 ++-
> src/app/models/instance.rb | 2 +-
> src/app/models/pool_family.rb | 14 ++++++++++++++
> src/app/models/provider_account.rb | 1 -
> 4 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/src/app/models/deployment.rb b/src/app/models/deployment.rb
> index fe02ad3..cd2eb73 100644
> --- a/src/app/models/deployment.rb
> +++ b/src/app/models/deployment.rb
> @@ -491,7 +491,8 @@ class Deployment < ActiveRecord::Base
> errors = e.map {|e| "#{instance.name}: #{e}"}
> m
> end
> - pool.pool_family.provider_accounts.ascending_by_priority.each do |account|
> +
> + pool.pool_family.provider_accounts_by_priority.each do |account|
> matches_by_account = all_matches.map do |m|
> m.find {|m| m.provider_account.id == account.id}
> end
> diff --git a/src/app/models/instance.rb b/src/app/models/instance.rb
> index 60a0f09..95e2717 100644
> --- a/src/app/models/instance.rb
> +++ b/src/app/models/instance.rb
> @@ -403,7 +403,7 @@ class Instance < ActiveRecord::Base
> return [[], errors] unless errors.empty?
>
> matched = []
> - pool.pool_family.provider_accounts.ascending_by_priority.each do |account|
> + pool.pool_family.provider_accounts_by_priority.each do |account|
> account.instance_matches(self, matched, errors)
> end
>
> diff --git a/src/app/models/pool_family.rb b/src/app/models/pool_family.rb
> index 8f5ea24..dd474a9 100644
> --- a/src/app/models/pool_family.rb
> +++ b/src/app/models/pool_family.rb
> @@ -70,6 +70,20 @@ class PoolFamily < ActiveRecord::Base
> !provider_accounts.empty? and !provider_accounts.any? {|acc| acc.provider.enabled?}
> end
>
> + def provider_accounts_by_priority
> + provider_accounts.sort {|a,b|
> + if a.priority.nil? and b.priority.nil?
> + 0
> + elsif a.priority.nil?
> + 1
> + elsif b.priority.nil?
> + -1
> + else
> + a.priority <=> b.priority
> + end
> + }
> + end
> +
Another "for the record" based on our IRC discussion -- I really, really
don't like this, since we're moving a trivial database operation into
our code base, which feels like it's asking for trouble. But the
reasoning here is compelling enough that I'll go along with it --
sorting on tables containing nils (as is the case with priority)
apparently doesn't behave consistently across databases.
> def statistics
> max = quota.maximum_running_instances
> total = quota.running_instances
> diff --git a/src/app/models/provider_account.rb b/src/app/models/provider_account.rb
> index 9647dd5..d0323a9 100644
> --- a/src/app/models/provider_account.rb
> +++ b/src/app/models/provider_account.rb
> @@ -73,7 +73,6 @@ class ProviderAccount < ActiveRecord::Base
> before_destroy :destroyable?
>
> scope :enabled, lambda { where(:provider_id => Provider.enabled) }
> - scope :ascending_by_priority, :order => 'provider_accounts.priority ASC'
>
> # We set credentials hash as protected so that it is not set during mass assign on new
> # This is to avoid the scenario where the credentials are set before provider which
> --
> 1.7.6.5
ACK, with inline comment.
-- Matt
More information about the aeolus-devel
mailing list