[PATCH] BZ 784479 add range validation to priority
Tzu-Mainn Chen
tzumainn at redhat.com
Thu Mar 1 19:57:16 UTC 2012
Okay, tested with the new limits, and it works as expected. Patch updated and pushed to master. Thanks!
Mainn
----- Original Message -----
From: "Matt Wagner" <matt.wagner at redhat.com>
To: "Tzu-Mainn Chen" <tzumainn at redhat.com>
Cc: aeolus-devel at lists.fedorahosted.org
Sent: Thursday, March 1, 2012 2:54:37 PM
Subject: Re: [PATCH] BZ 784479 add range validation to priority
On Thu, Feb 23, 2012 at 12:54:20PM -0500, Tzu-Mainn Chen wrote:
> ---
> src/app/models/provider_account.rb | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/app/models/provider_account.rb b/src/app/models/provider_account.rb
> index eff59f9..49f8e67 100644
> --- a/src/app/models/provider_account.rb
> +++ b/src/app/models/provider_account.rb
> @@ -66,7 +66,7 @@ class ProviderAccount < ActiveRecord::Base
> validate :validate_presence_of_credentials
> validate :validate_credentials
> validate :validate_unique_username
> - validates :priority, :numericality => { :only_integer => true}, :allow_blank => true
> + validates :priority, :numericality => { :only_integer => true, :greater_than_or_equal_to => -100000, :less_than_or_equal_to => 100000}, :allow_blank => true
>
> before_create :populate_profiles_and_validate
> after_create :populate_realms_and_validate
> --
> 1.7.6.5
ACK-ish.
This looks good and works just fine. However, the +/- 100,000 limit
feels arbitrary. I'm not sure if anyone will actually be setting up
provider accounts with a priority of 123,456, but if the goal is just to
prevent overflowing the field, I wonder if we can accept a greater
range.
quota.rb defines a range of 0-2147483647. I bet we could support
-2147483647 to +2147483647.
So somewhere between an ACK and a conditional ACK -- please consider the
above change if it makes sense to you.
-- Matt
More information about the aeolus-devel
mailing list