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

[Zhou Zheng Sheng]

on 2014/04/02 03:25, 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
> 
Sorting is for user friendly presentation which is the responsibility of
the controller or even the front-end.

Considering the sorting efficiency, I think the object size of the
element in the list can be ignored because the list element might be
just a reference or a pointer to the real object. The significant factor
of the sorting time and memory is the length of the list. In this case,
the length does not change. So there is no big efficiency overhead to
put it in controller. A valid reason to put the sorting in
disks.get_partitions_name() is that the sorting code is re-used by
anyone calling disks.get_partitions_name(). But actually we are not sure
if the caller needs a sorted list.

Sorting in either controller or disks.get_partitions_name() is OK to me,
while the former is better.

>> 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):
>>>
>>
> 
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/kimchi-devel
> 


-- 
Thanks and best regards!

Zhou Zheng Sheng / 周征晟
E-mail: zhshzhou at linux.vnet.ibm.com
Telephone: 86-10-82454397