Re: [openstack-dev] [ ceilometer] The Pagination patches
On Fri, 2014-01-17 at 06:59 +, Gao, Fengqian wrote: Hi, Jay, Thanks for your reply. According to you review comments, I rebase my code, please see my comments for your questions. For the issue that ensure there is at least one unique column in the sort keys if pagination is used, I have an idea. How about add the primary key at the end of sort keys list passed from users? Thanks, Fengqian, Yes, that is exactly what I was thinking... make a helper method that wraps paginate_query() and looks at the sort_keys and, if there is no column that is unique, it will append the primary key column(s) to the sort keys. I'll review your latest patches later today (currently up in Montreal at the Neutron/Tempest QA code sprint, so a little delayed...) Best, -jay --fengqian -Original Message- From: Jay Pipes [mailto:jaypi...@gmail.com] Sent: Monday, January 13, 2014 5:10 AM To: Gao, Fengqian Cc: Julien Danjou (jul...@danjou.info); Doug Hellmann (doug.hellm...@dreamhost.com); Lu, Lianhao; chu...@ca.ibm.com; dper...@linux.vnet.ibm.com; akornie...@mirantis.com; openstack-dev@lists.openstack.org Subject: Re: [openstack-dev][ ceilometer] The Pagination patches On Fri, 2014-01-10 at 03:31 +, Gao, Fengqian wrote: Since we still have no new conclusion for pagination for now, shall we still go on with the current pagination solution? Please help to review patches: https://review.openstack.org/#/c/41869/ https://review.openstack.org/#/c/35454/ Hi Fengqian, Please see my review on the first of the above patches. You need to ensure that there is at least one unique column in the sort keys if pagination is used. There is a similar issue in the second patch (reviewing it now) that uses the sqlalchemy_utils (oslo.db) paginate_query() method... The sqlalchemy_utils.paginate_query() method **requires** that sort_keys contains a unique column [1]. The docstring for that method says: Pagination works by requiring a unique sort_key, specified by sort_keys. (If sort_keys is not unique, then we risk looping through values.) So, care must be taken to pass sort_keys parameter to paginate_query() that already contains at least one unique column in the sort keys. Best, -jay [1] https://github.com/openstack/oslo-incubator/blob/master/openstack/common/db/sqlalchemy/utils.py#L66 ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [ ceilometer] The Pagination patches
Hi, Jay, Thanks for your reply. According to you review comments, I rebase my code, please see my comments for your questions. For the issue that ensure there is at least one unique column in the sort keys if pagination is used, I have an idea. How about add the primary key at the end of sort keys list passed from users? Thanks --fengqian -Original Message- From: Jay Pipes [mailto:jaypi...@gmail.com] Sent: Monday, January 13, 2014 5:10 AM To: Gao, Fengqian Cc: Julien Danjou (jul...@danjou.info); Doug Hellmann (doug.hellm...@dreamhost.com); Lu, Lianhao; chu...@ca.ibm.com; dper...@linux.vnet.ibm.com; akornie...@mirantis.com; openstack-dev@lists.openstack.org Subject: Re: [openstack-dev][ ceilometer] The Pagination patches On Fri, 2014-01-10 at 03:31 +, Gao, Fengqian wrote: Since we still have no new conclusion for pagination for now, shall we still go on with the current pagination solution? Please help to review patches: https://review.openstack.org/#/c/41869/ https://review.openstack.org/#/c/35454/ Hi Fengqian, Please see my review on the first of the above patches. You need to ensure that there is at least one unique column in the sort keys if pagination is used. There is a similar issue in the second patch (reviewing it now) that uses the sqlalchemy_utils (oslo.db) paginate_query() method... The sqlalchemy_utils.paginate_query() method **requires** that sort_keys contains a unique column [1]. The docstring for that method says: Pagination works by requiring a unique sort_key, specified by sort_keys. (If sort_keys is not unique, then we risk looping through values.) So, care must be taken to pass sort_keys parameter to paginate_query() that already contains at least one unique column in the sort keys. Best, -jay [1] https://github.com/openstack/oslo-incubator/blob/master/openstack/common/db/sqlalchemy/utils.py#L66 ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
Re: [openstack-dev] [ ceilometer] The Pagination patches
On Fri, 2014-01-10 at 03:31 +, Gao, Fengqian wrote: Since we still have no new conclusion for pagination for now, shall we still go on with the current pagination solution? Please help to review patches: https://review.openstack.org/#/c/41869/ https://review.openstack.org/#/c/35454/ Hi Fengqian, Please see my review on the first of the above patches. You need to ensure that there is at least one unique column in the sort keys if pagination is used. There is a similar issue in the second patch (reviewing it now) that uses the sqlalchemy_utils (oslo.db) paginate_query() method... The sqlalchemy_utils.paginate_query() method **requires** that sort_keys contains a unique column [1]. The docstring for that method says: Pagination works by requiring a unique sort_key, specified by sort_keys. (If sort_keys is not unique, then we risk looping through values.) So, care must be taken to pass sort_keys parameter to paginate_query() that already contains at least one unique column in the sort keys. Best, -jay [1] https://github.com/openstack/oslo-incubator/blob/master/openstack/common/db/sqlalchemy/utils.py#L66 ___ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
