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(a)linux.vnet.ibm.com&gt; --- 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 []

class Partition(Resource): def __init__(self, model, id):

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(a)linux.vnet.ibm.com&gt; >> --- >> 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().

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): >

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(a)linux.vnet.ibm.com&gt; >>>> --- >>>> 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(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/kimchi-devel >