[Ovirt-devel] [PATCH 4/6] hardware_pool: search by path

[Scott Seago]

David Lutterkort wrote:
> On Mon, 2008-08-18 at 09:25 -0400, Scott Seago wrote:
>   
>> David Lutterkort wrote:
>>     
>>> Signed-off-by: David Lutterkort <dlutter at redhat.com>
>>> ---
>>>  wui/src/app/models/hardware_pool.rb |   12 ++++++++++++
>>>  1 files changed, 12 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/wui/src/app/models/hardware_pool.rb b/wui/src/app/models/hardware_pool.rb
>>> index 276779f..249d744 100644
>>> --- a/wui/src/app/models/hardware_pool.rb
>>> +++ b/wui/src/app/models/hardware_pool.rb
>>> @@ -97,4 +97,16 @@ class HardwarePool < Pool
>>>      return {:total => total, :labels => labels}
>>>    end
>>>  
>>> +  def self.find_by_path(path)
>>> +      segs = path.split("/")
>>> +      unless segs.shift.empty?
>>> +          raise "Path must be absolute, but is #{path}"
>>> +      end
>>> +      if segs.shift == "default"
>>> +          segs.inject(get_default_pool) do |pool, seg|
>>> +              pool.sub_hardware_pools.find { |p| p.name == seg } if pool
>>> +          end
>>> +      end
>>> +  end
>>> +
>>>  end
>>>   
>>>       
>> We shouldn't assume that the default pool happens to be named 'default'. 
>> I'm not sure the best way to do this, but one way is to treat the 
>> default pool as having path "/". So Pool.find_by_path("/") would return 
>> the default pool, and Pool.find_by_path("/engineering/QA") would start 
>> with the default pool, find a subpool named "engineering" and a subpool 
>> of engineering called "QA".
>>     
>
> That won't work .. if '/' is the default pool, what is '/default' ?
>
> In the above code, I can get rid of the hardcoded "default" by comparing
> against get_default_pool.name.
>
> David
>
>   
We could probably make '/' work for the default pool, but it looks like 
it might be too confusing. If '/' were the default pool, then '/default' 
would be a subpool of 'default' which happens to be called default -- 
but I like the other idea better anyway. i.e. the first pool in the path 
after the initial '/' is compared against the default pool.

I'm assuming the path-based pool lookup is just an alternate method of 
getting this from your API, as the id-based ones will all still work. I 
just realized that full path-based lookup will only work for users that 
have read permissions on the whole hierarchy. A user with lower-level 
permissions only (i.e. only read permissions for pools under 
'/default/engineering/qa' and write permissions for subpools below that) 
won't even see the top level pool. In   the WUI, the left-hand nav tree 
begins with the pools that a user has read permissions on -- other pools 
don't even show up. So for such users, the full path lookup won't be so 
useful.

Scott