From 356df24c14746e1fa629d1bc207b01a25f73857e Mon Sep 17 00:00:00 2001 From: Anirudh Bagri Date: Wed, 3 Mar 2021 15:40:50 +0530 Subject: [PATCH 01/12] Add support for limit and offset in list_values method --- st2client/st2client/models/core.py | 6 +++++- st2common/st2common/services/datastore.py | 10 ++++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py index e6fec6da70..63f8b4c77c 100644 --- a/st2client/st2client/models/core.py +++ b/st2client/st2client/models/core.py @@ -171,10 +171,11 @@ def handle_error(response): def get_all(self, **kwargs): # TODO: This is ugly, stop abusing kwargs url = '/%s' % self.resource.get_url_path_name() - limit = kwargs.pop('limit', None) + limit = kwargs.pop('limit', 100) pack = kwargs.pop('pack', None) prefix = kwargs.pop('prefix', None) user = kwargs.pop('user', None) + offset = kwargs.pop('offset', 0) params = kwargs.pop('params', {}) @@ -190,6 +191,9 @@ def get_all(self, **kwargs): if user: params['user'] = user + if offset: + params['offset'] = user + response = self.client.get(url=url, params=params, **kwargs) if response.status_code != http_client.OK: self.handle_error(response) diff --git a/st2common/st2common/services/datastore.py b/st2common/st2common/services/datastore.py index 986ffd0d03..340c4319b7 100644 --- a/st2common/st2common/services/datastore.py +++ b/st2common/st2common/services/datastore.py @@ -72,7 +72,7 @@ def get_user_info(self): # Methods for datastore management ################################## - def list_values(self, local=True, prefix=None): + def list_values(self, local=True, prefix=None, limit=100, offset=0): """ Retrieve all the datastores items. @@ -82,13 +82,19 @@ def list_values(self, local=True, prefix=None): :param prefix: Optional key name prefix / startswith filter. :type prefix: ``str`` + :param limit: Number of keys to get. Defaults to 100. + :type limit: ``integer`` + + :param offset: Number of keys to offset. Defaults to 0. + :type offset: ``integer`` + :rtype: ``list`` of :class:`KeyValuePair` """ client = self.get_api_client() self._logger.debug('Retrieving all the values from the datastore') key_prefix = self._get_full_key_prefix(local=local, prefix=prefix) - kvps = client.keys.get_all(prefix=key_prefix) + kvps = client.keys.get_all(prefix=key_prefix, limit=limit, offset=offset) return kvps def get_value(self, name, local=True, scope=SYSTEM_SCOPE, decrypt=False): From 58ecb329875c7e2a92942b9afed9fefdf983ef67 Mon Sep 17 00:00:00 2001 From: Anirudh Bagri Date: Mon, 8 Mar 2021 11:17:03 +0530 Subject: [PATCH 02/12] reformatted --- st2client/st2client/models/core.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py index af5480d378..edd6d38484 100644 --- a/st2client/st2client/models/core.py +++ b/st2client/st2client/models/core.py @@ -175,13 +175,13 @@ def handle_error(response): @add_auth_token_to_kwargs_from_env def get_all(self, **kwargs): # TODO: This is ugly, stop abusing kwargs - url = '/%s' % self.resource.get_url_path_name() - limit = kwargs.pop('limit', 100) - pack = kwargs.pop('pack', None) - prefix = kwargs.pop('prefix', None) - user = kwargs.pop('user', None) - offset = kwargs.pop('offset', 0) - + url = "/%s" % self.resource.get_url_path_name() + limit = kwargs.pop("limit", 100) + pack = kwargs.pop("pack", None) + prefix = kwargs.pop("prefix", None) + user = kwargs.pop("user", None) + offset = kwargs.pop("offset", 0) + params = kwargs.pop("params", {}) if limit: @@ -197,7 +197,7 @@ def get_all(self, **kwargs): params["user"] = user if offset: - params['offset'] = user + params["offset"] = user response = self.client.get(url=url, params=params, **kwargs) if response.status_code != http_client.OK: From 6db19eaafe7276de32ea8598693e20184f3df183 Mon Sep 17 00:00:00 2001 From: Anirudh Bagri Date: Tue, 9 Mar 2021 10:19:13 +0530 Subject: [PATCH 03/12] Updated core.py to use None as default. --- st2client/st2client/models/core.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py index edd6d38484..04c9fc786a 100644 --- a/st2client/st2client/models/core.py +++ b/st2client/st2client/models/core.py @@ -176,11 +176,11 @@ def handle_error(response): def get_all(self, **kwargs): # TODO: This is ugly, stop abusing kwargs url = "/%s" % self.resource.get_url_path_name() - limit = kwargs.pop("limit", 100) + limit = kwargs.pop("limit", None) pack = kwargs.pop("pack", None) prefix = kwargs.pop("prefix", None) user = kwargs.pop("user", None) - offset = kwargs.pop("offset", 0) + offset = kwargs.pop("offset", None) params = kwargs.pop("params", {}) @@ -197,7 +197,7 @@ def get_all(self, **kwargs): params["user"] = user if offset: - params["offset"] = user + params["offset"] = offset response = self.client.get(url=url, params=params, **kwargs) if response.status_code != http_client.OK: From 943da9db6ccdd39264e9e9f6da5be15190fe602d Mon Sep 17 00:00:00 2001 From: Anirudh Bagri Date: Tue, 9 Mar 2021 10:24:30 +0530 Subject: [PATCH 04/12] Getting value of limit from the configuration. --- st2common/st2common/services/datastore.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/st2common/st2common/services/datastore.py b/st2common/st2common/services/datastore.py index 1026d112c4..d87794a9f2 100644 --- a/st2common/st2common/services/datastore.py +++ b/st2common/st2common/services/datastore.py @@ -68,7 +68,7 @@ def get_user_info(self): # Methods for datastore management ################################## - def list_values(self, local=True, prefix=None, limit=100, offset=0): + def list_values(self, local=True, prefix=None, limit=None, offset=0): """ Retrieve all the datastores items. @@ -78,7 +78,7 @@ def list_values(self, local=True, prefix=None, limit=100, offset=0): :param prefix: Optional key name prefix / startswith filter. :type prefix: ``str`` - :param limit: Number of keys to get. Defaults to 100. + :param limit: Number of keys to get. Defaults to the configuration set at 'api.max_page_size'. :type limit: ``integer`` :param offset: Number of keys to offset. Defaults to 0. @@ -89,6 +89,7 @@ def list_values(self, local=True, prefix=None, limit=100, offset=0): client = self.get_api_client() self._logger.debug("Retrieving all the values from the datastore") + limit = limit or cfg.CONF.api.max_page_size key_prefix = self._get_full_key_prefix(local=local, prefix=prefix) kvps = client.keys.get_all(prefix=key_prefix, limit=limit, offset=offset) return kvps From a6cd1b4069e34a907e5f3b1163ccf9866e7e003d Mon Sep 17 00:00:00 2001 From: Anirudh Bagri Date: Tue, 9 Mar 2021 10:26:11 +0530 Subject: [PATCH 05/12] Setting default value of offset to 0 --- st2client/st2client/models/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2client/st2client/models/core.py b/st2client/st2client/models/core.py index 04c9fc786a..66caec1626 100644 --- a/st2client/st2client/models/core.py +++ b/st2client/st2client/models/core.py @@ -180,7 +180,7 @@ def get_all(self, **kwargs): pack = kwargs.pop("pack", None) prefix = kwargs.pop("prefix", None) user = kwargs.pop("user", None) - offset = kwargs.pop("offset", None) + offset = kwargs.pop("offset", 0) params = kwargs.pop("params", {}) From a6ff2b7b514366e74ac6f31ed8dc45e317951d9c Mon Sep 17 00:00:00 2001 From: Anirudh Bagri Date: Tue, 9 Mar 2021 11:31:07 +0530 Subject: [PATCH 06/12] fix and add tests for datastore --- st2common/tests/unit/test_datastore.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/st2common/tests/unit/test_datastore.py b/st2common/tests/unit/test_datastore.py index 1e3dc86d30..a1900c1d99 100644 --- a/st2common/tests/unit/test_datastore.py +++ b/st2common/tests/unit/test_datastore.py @@ -51,14 +51,14 @@ def test_datastore_operations_list_values(self): self._set_mock_api_client(mock_api_client) self._datastore_service.list_values(local=True, prefix=None) - mock_api_client.keys.get_all.assert_called_with(prefix="core.TestSensor:") + mock_api_client.keys.get_all.assert_called_with(prefix="core.TestSensor:", limit=100, offset=0) self._datastore_service.list_values(local=True, prefix="ponies") - mock_api_client.keys.get_all.assert_called_with(prefix="core.TestSensor:ponies") + mock_api_client.keys.get_all.assert_called_with(prefix="core.TestSensor:ponies", limit=100, offset=0) self._datastore_service.list_values(local=False, prefix=None) - mock_api_client.keys.get_all.assert_called_with(prefix=None) + mock_api_client.keys.get_all.assert_called_with(prefix=None, limit=100, offset=0) self._datastore_service.list_values(local=False, prefix="ponies") - mock_api_client.keys.get_all.assert_called_with(prefix="ponies") + mock_api_client.keys.get_all.assert_called_with(prefix="ponies", limit=100, offset=0) # No values in the datastore mock_api_client = mock.Mock() @@ -85,6 +85,14 @@ def test_datastore_operations_list_values(self): self.assertEqual(len(values), 2) self.assertEqual(values, mock_return_value) + # Test limit + values = self._datastore_service.list_values(local=True, limit=1) + self.assertEqual(len(values), 1) + + # Test offset + values = self._datastore_service.list_values(local=True, offset=1) + self.assertEqual(len(values), 1) + def test_datastore_operations_get_value(self): mock_api_client = mock.Mock() kvp1 = KeyValuePair() From cc0922e494c1c74f92c18f6854a2e51889bc4982 Mon Sep 17 00:00:00 2001 From: Anirudh Bagri Date: Tue, 9 Mar 2021 11:39:48 +0530 Subject: [PATCH 07/12] updated changelog --- CHANGELOG.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 0fa0a562b8..30c16c865a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -4,6 +4,13 @@ Changelog in development -------------- +Added +~~~~~~~ + +* Added support for limit and offset in list_values method (#5097 and #5171.) + + Contributed by @anirudhbagri. + Changed ~~~~~~~ From 3d76db80064c6889d036477dd17bf0b7f1c18bfd Mon Sep 17 00:00:00 2001 From: Anirudh Bagri Date: Tue, 9 Mar 2021 11:41:22 +0530 Subject: [PATCH 08/12] black changes --- st2common/tests/unit/test_datastore.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/st2common/tests/unit/test_datastore.py b/st2common/tests/unit/test_datastore.py index a1900c1d99..aa58f75497 100644 --- a/st2common/tests/unit/test_datastore.py +++ b/st2common/tests/unit/test_datastore.py @@ -51,14 +51,22 @@ def test_datastore_operations_list_values(self): self._set_mock_api_client(mock_api_client) self._datastore_service.list_values(local=True, prefix=None) - mock_api_client.keys.get_all.assert_called_with(prefix="core.TestSensor:", limit=100, offset=0) + mock_api_client.keys.get_all.assert_called_with( + prefix="core.TestSensor:", limit=100, offset=0 + ) self._datastore_service.list_values(local=True, prefix="ponies") - mock_api_client.keys.get_all.assert_called_with(prefix="core.TestSensor:ponies", limit=100, offset=0) + mock_api_client.keys.get_all.assert_called_with( + prefix="core.TestSensor:ponies", limit=100, offset=0 + ) self._datastore_service.list_values(local=False, prefix=None) - mock_api_client.keys.get_all.assert_called_with(prefix=None, limit=100, offset=0) + mock_api_client.keys.get_all.assert_called_with( + prefix=None, limit=100, offset=0 + ) self._datastore_service.list_values(local=False, prefix="ponies") - mock_api_client.keys.get_all.assert_called_with(prefix="ponies", limit=100, offset=0) + mock_api_client.keys.get_all.assert_called_with( + prefix="ponies", limit=100, offset=0 + ) # No values in the datastore mock_api_client = mock.Mock() From 8546730ee27f66d4a765499c2aa8202240d70bf9 Mon Sep 17 00:00:00 2001 From: Anirudh Bagri Date: Tue, 9 Mar 2021 12:26:34 +0530 Subject: [PATCH 09/12] fix test --- st2common/tests/unit/test_datastore.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/st2common/tests/unit/test_datastore.py b/st2common/tests/unit/test_datastore.py index aa58f75497..2690e1db99 100644 --- a/st2common/tests/unit/test_datastore.py +++ b/st2common/tests/unit/test_datastore.py @@ -94,12 +94,16 @@ def test_datastore_operations_list_values(self): self.assertEqual(values, mock_return_value) # Test limit - values = self._datastore_service.list_values(local=True, limit=1) - self.assertEqual(len(values), 1) + _ = self._datastore_service.list_values(local=True, limit=1) + mock_api_client.keys.get_all.assert_called_with( + prefix=None, limit=1, offset=0 + ) # Test offset - values = self._datastore_service.list_values(local=True, offset=1) - self.assertEqual(len(values), 1) + _ = self._datastore_service.list_values(local=True, offset=1) + mock_api_client.keys.get_all.assert_called_with( + prefix=None, limit=100, offset=1 + ) def test_datastore_operations_get_value(self): mock_api_client = mock.Mock() From 336cc1da15c6f65c6f8e5f467279f5f23dd17320 Mon Sep 17 00:00:00 2001 From: Anirudh Bagri Date: Tue, 9 Mar 2021 12:30:24 +0530 Subject: [PATCH 10/12] black changes --- st2common/tests/unit/test_datastore.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/st2common/tests/unit/test_datastore.py b/st2common/tests/unit/test_datastore.py index 2690e1db99..33a7ea0cfb 100644 --- a/st2common/tests/unit/test_datastore.py +++ b/st2common/tests/unit/test_datastore.py @@ -95,9 +95,7 @@ def test_datastore_operations_list_values(self): # Test limit _ = self._datastore_service.list_values(local=True, limit=1) - mock_api_client.keys.get_all.assert_called_with( - prefix=None, limit=1, offset=0 - ) + mock_api_client.keys.get_all.assert_called_with(prefix=None, limit=1, offset=0) # Test offset _ = self._datastore_service.list_values(local=True, offset=1) From 816a56ddc37db3bd6ba45a0f107ee558bb5fcbb1 Mon Sep 17 00:00:00 2001 From: Anirudh Bagri Date: Tue, 9 Mar 2021 12:38:08 +0530 Subject: [PATCH 11/12] fix tests --- st2common/tests/unit/test_datastore.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/st2common/tests/unit/test_datastore.py b/st2common/tests/unit/test_datastore.py index 33a7ea0cfb..9827aafd30 100644 --- a/st2common/tests/unit/test_datastore.py +++ b/st2common/tests/unit/test_datastore.py @@ -95,12 +95,14 @@ def test_datastore_operations_list_values(self): # Test limit _ = self._datastore_service.list_values(local=True, limit=1) - mock_api_client.keys.get_all.assert_called_with(prefix=None, limit=1, offset=0) + mock_api_client.keys.get_all.assert_called_with( + prefix="core.TestSensor:", limit=1, offset=0 + ) # Test offset _ = self._datastore_service.list_values(local=True, offset=1) mock_api_client.keys.get_all.assert_called_with( - prefix=None, limit=100, offset=1 + prefix="core.TestSensor:", limit=100, offset=1 ) def test_datastore_operations_get_value(self): From 49b7f9a42d5aeeecff0f0ffe00df4001bb512402 Mon Sep 17 00:00:00 2001 From: Tomaz Muraus Date: Fri, 12 Mar 2021 11:13:00 +0100 Subject: [PATCH 12/12] Update CHANGELOG.rst --- CHANGELOG.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 30c16c865a..3766ce2ad1 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -7,7 +7,8 @@ in development Added ~~~~~~~ -* Added support for limit and offset in list_values method (#5097 and #5171.) +* Added support for ``limit`` and ``offset`` argument to the ``list_values`` data store + service method (#5097 and #5171). Contributed by @anirudhbagri.