[PATCH V2] Sort device paths shown when creating a logical storage pool
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@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 [] class Partition(Resource): def __init__(self, model, id): -- 1.8.5.3
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@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? 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.
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@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(). 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 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@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
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@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@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
-- Thanks and best regards! Zhou Zheng Sheng / 周征晟 E-mail: zhshzhou@linux.vnet.ibm.com Telephone: 86-10-82454397
On 04/02/2014 05:30 AM, Zhou Zheng Sheng wrote:
on 2014/04/02 03:25, Aline Manera wrote:
On 04/01/2014 03:13 PM, Aline Manera 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@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
On 03/27/2014 11:12 PM, Rodrigo Trujillo wrote: 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
On 04/01/2014 04:21 PM, Rodrigo Trujillo wrote: 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 V4
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@ovirt.org http://lists.ovirt.org/mailman/listinfo/kimchi-devel
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@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):
participants (3)
-
Aline Manera -
Rodrigo Trujillo -
Zhou Zheng Sheng