On 07/22/2012 12:16 PM, [email protected] wrote:
This patchset implements the initial framework
Hi Imre,
this is not a proper review (Jozef is reviewing this patch AFAIK), here
is a few things we discussed on Friday:
- provider account's priority is not used when this patch is applied,
it's replaced with priority groups. I'm fine with your suggestion that
this account's priority can be used for setting wigh of account when
choosing random account inside priority group.
- provider_priority_groups controller is missing permissions checking
for some actions
- priority group is not destroyed when the pool is destroyed, priority
element is not destroyed when account or provider is destroyed
- config/initializers/provider_selection_strategies.rb: only
lib/provider.rb could be loaded instead of the each loop
- I wonder if priority groups shouldn't be associated with a pool family
isntead of a pool since provider accounts are associated with pool
family, not pool?
- you use reverse priority ordering (bigger = higher) than what is used
for account's priority (lower = higher), we should unify this
In future (post 1.1) it would be nice to make provider_selection library
more abstract (conductor independent) which can be reused in other apps.
The conductor specific logic would be just inside strategies.
The above are mostly minor notes, overall I think this is a good stuff
and it can be pushed before 1.1.
Great job, Jan
