[Kimchi-devel] [PATCH V2] Sort device paths shown when creating a logical storage pool

[Rodrigo Trujillo]

On 04/01/2014 04:25 PM, Aline Manera wrote:
> On 04/01/2014 04:21 PM, Rodrigo Trujillo wrote:
>> On 04/01/2014 03:13 PM, Aline Manera wrote:
>>> On 03/27/2014 11:12 PM, Rodrigo Trujillo wrote:
>>>> This patch sorts the host partitions list returned by backend by
>>>> partition path. Then UI is going to show paths sorted, improving
>>>> the user experience.
>>>>
>>>> Signed-off-by: Rodrigo Trujillo <rodrigo.trujillo at linux.vnet.ibm.com>
>>>> ---
>>>>   src/kimchi/control/host.py | 20 +++++++++++++++++++-
>>>>   1 file changed, 19 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/src/kimchi/control/host.py b/src/kimchi/control/host.py
>>>> index cfc24bd..d4387f4 100644
>>>> --- a/src/kimchi/control/host.py
>>>> +++ b/src/kimchi/control/host.py
>>>> @@ -20,7 +20,7 @@
>>>>   import cherrypy
>>>>
>>>>   from kimchi.control.base import Collection, Resource
>>>> -from kimchi.control.utils import UrlSubNode, validate_method
>>>> +from kimchi.control.utils import UrlSubNode, validate_method, 
>>>> model_fn
>>>>   from kimchi.exception import OperationFailed
>>>>   from kimchi.template import render
>>>>
>>>> @@ -64,6 +64,24 @@ class Partitions(Collection):
>>>>           super(Partitions, self).__init__(model)
>>>>           self.resource = Partition
>>>>
>>>> +    # Defining get_resources in order to return list of partitions 
>>>> in UI
>>>> +    # sorted by their path
>>>> +    def _get_resources(self, flag_filter):
>>>> +        try:
>>>> +            get_list = getattr(self.model, model_fn(self, 
>>>> 'get_list'))
>>>> +            idents = get_list(*self.model_args, **flag_filter)
>>>> +            res_list = []
>>>> +            for ident in idents:
>>>> +                # internal text, get_list changes ident to unicode 
>>>> for sorted
>>>> +                args = self.resource_args + [ident]
>>>> +                res = self.resource(self.model, *args)
>>>> +                res.lookup()
>>>> +                res_list.append(res)
>>>> +            # Sort by partition path
>>>> +            res_list.sort(key=lambda x: x.info['path'])
>>>> +            return res_list
>>>> +        except AttributeError:
>>>> +            return []
>>>
>>> Wow! So you will get all resources for then sort them?
>>> Why don't change Partitions.get_list() to return the sorted list?
>>> get_partitions_names
>>> If you check disks.get_partitions_names() it already contains the 
>>> device path, you just need to sort
>>> the list before returning
>>>
>>> So disks.get_partitions_names() will return the list sorted by path.
>>
>> I do not see path in disks.get_partitions_names() ... and If I do 
>> this I would have to sort a bigger list than in _get_resources().
>
> The disks.get_partitions_name() is what provide the result for get() 
> so the list is not bigger,
>
> Take a look at disks.py line 146

I see. But the "path" is only used in the "if"... it is not part of the 
data structure.
Does not make sense return a list of "names" (only "names") sorted by 
"path".
You are asking to create a new structure, with name and path, sort, 
remove path and return the names.

Please, see Zhou's email and V4. Let me know if it works.

>
>> Please, see the version V4 of this patch ... I think it has the best 
>> approach to solve the problem.
>>
>>
>>>
>>>>   class Partition(Resource):
>>>>       def __init__(self, model, id):
>>>
>>
>