Add support for limit and offset in list_values method by anirudhbagri · Pull Request #5176 · StackStorm/st2

anirudhbagri · 2021-03-03T10:13:05Z

Added ability to provide limit and offset for list_values().

Partially fixes #5097 and fixes #5171.

cognifloyd · 2021-03-07T07:22:37Z

We just reformatted the code with black. (Hooray!) And this PR got caught in the cross fire too. (Arrgh!)
Please merge master into your branch, resolve the conflicts (Ouch! Sorry!), and reformat with black (I recommend running pre-commit install after you've merged master so that black reformatting happens automatically on commit).

arm4b · 2021-03-08T23:17:03Z

@cognifloyd Can you please also point @anirudhbagri in the right direction to finish this PR?

cognifloyd

I'm just reviewing this. My first question was, what defines the default limit? It looks like it's defined in config:

st2/conf/st2.conf.sample

Lines 43 to 44 in 56101b8

    
           # Maximum limit (page size) argument which can be specified by the user in a query string. 
        
           max_page_size = 100

st2/st2api/st2api/config.py

Lines 84 to 89 in 8496bb2

    
           cfg.IntOpt( 
        
               "max_page_size", 
        
               default=100, 
        
               help="Maximum limit (page size) argument which can be " 
        
               "specified by the user in a query string.", 
        
           ),

st2/st2api/st2api/controllers/resource.py

Lines 126 to 129 in 8496bb2

    
           # Maximum value of limit which can be specified by user 
        
           @property 
        
           def max_limit(self): 
        
               return cfg.CONF.api.max_page_size

So, could we use the default from the config?

st2client/st2client/models/core.py

st2common/st2common/services/datastore.py

cognifloyd · 2021-03-09T05:13:04Z

Let's see. I think we'll also need some tests for this feature. It looks like this is where the list_values() tests are:

st2/st2common/tests/unit/test_datastore.py

Line 47 in 8496bb2

def test_datastore_operations_list_values(self):

Oh, and I guess that's also the test that is failing: https://github.com/StackStorm/st2/pull/5176/checks?check_run_id=2063056481#step:14:2287

Plus, I think we'll need a test for the st2client bit.

cognifloyd · 2021-03-09T05:15:13Z

Oh. And I often forget adding a changelog entry. Please do that too. Then I think this will be good to merge :)

arm4b

Thanks, @anirudhbagri for the contribution and @cognifloyd for review and making this PR complete 👍

@Kami would also appreciate your final 👀 and ✔️ as someone involved in previous pagination discussions around this specific issue.

cognifloyd

LGTM

Kami · 2021-03-12T10:10:22Z

st2client/st2client/models/core.py

        if user:
            params["user"] = user

+        if offset:


Since if we use a truthy check, this means offset won't be included if set to 0, but yeah I think this should be fine since that's the current / existing default behavior if offset is not specified.

glafkosch · 2023-04-15T13:28:45Z

I have upgraded to latest Stackstorm 3.8 version and I am getting also this issue. Any fix?

st2.actions.python.WorkflowCleanupAction: DEBUG Creating new Client object.\n\"auth.api_url\" configuration option is not configured\nst2.actions.python.WorkflowCleanupAction: DEBUG Retrieving all the values from the datastore\nTraceback (most recent call last):\n File \"/opt/stackstorm/st2/lib/python3.8/site-packages/oslo_config/cfg.py\", line 2262, in _get\n return self.__cache[key]\nKeyError: ('api', 'max_page_size')\n\nDuring handling of the above exception, another exception occurred:\n\nTraceback (most recent call last):\n File \"/opt/stackstorm/st2/lib/python3.8/site-packages/python_runner/python_action_wrapper.py\", line 395, in <module>\n obj.run()\n File \"/opt/stackstorm/st2/lib/python3.8/site-packages/python_runner/python_action_wrapper.py\", line 214, in run\n output = action.run(**self._parameters)\n File \"/opt/stackstorm/packs/tf/actions/workflow_cleanup.py\", line 14, in run\n items = self.action_service.list_values(local=False, prefix=key_prefix)\n File \"/opt/stackstorm/st2/lib/python3.8/site-packages/python_runner/python_action_wrapper.py\", line 136, in list_values\n return self.datastore_service.list_values(local=local, prefix=prefix)\n File \"/opt/stackstorm/st2/lib/python3.8/site-packages/st2common/services/datastore.py\", line 92, in list_values\n limit = limit or cfg.CONF.api.max_page_size\n File \"/opt/stackstorm/st2/lib/python3.8/site-packages/oslo_config/cfg.py\", line 2547, in __getattr__\n return self._conf._get(name, self._group)\n File \"/opt/stackstorm/st2/lib/python3.8/site-packages/oslo_config/cfg.py\", line 2264, in _get\n value = self._do_get(name, group, namespace)\n File \"/opt/stackstorm/st2/lib/python3.8/site-packages/oslo_config/cfg.py\", line 2282, in _do_get\n info = self._get_opt_info(name, group)\n File \"/opt/stackstorm/st2/lib/python3.8/site-packages/oslo_config/cfg.py\", line 2415, in _get_opt_info\n raise NoSuchOptError(opt_name, group)\noslo_config.cfg.NoSuchOptError: no such option in group api: max_page_size\n"

My workflow_clean.py

from st2common.runners.base_action import Action

class WorkflowCleanupAction(Action):
    def run(self, scan_id):
        key_prefix = f"tf.StoreWorkerResultAction:{scan_id}"
        items = self.action_service.list_values(local=False, prefix=key_prefix)
        for item in items:
            self.action_service.delete_value(item.name, local=False)

        return (True, None)

Temporary Fix

        #items = self.action_service.list_values(local=False, prefix=key_prefix)
        items = self.action_service.datastore_service.list_values(local=False, prefix=key_prefix, limit=100)

Add support for limit and offset in list_values method

356df24

pull-request-size bot added the size/S PR that changes 10-29 lines. Very easy to review. label Mar 3, 2021

Merge branch 'master' into master

068572b

anirudhbagri and others added 2 commits March 8, 2021 10:40

Merge branch 'master' into master

57e2ec3

reformatted

58ecb32

cognifloyd reviewed Mar 9, 2021

View reviewed changes

st2client/st2client/models/core.py Outdated Show resolved Hide resolved

st2client/st2client/models/core.py Outdated Show resolved Hide resolved

st2common/st2common/services/datastore.py Outdated Show resolved Hide resolved

anirudhbagri added 3 commits March 9, 2021 10:19

Updated core.py to use None as default.

6db19ea

Getting value of limit from the configuration.

943da9d

Setting default value of offset to 0

a6cd1b4

fix and add tests for datastore

a6ff2b7

pull-request-size bot added size/M PR that changes 30-99 lines. Good size to review. and removed size/S PR that changes 10-29 lines. Very easy to review. labels Mar 9, 2021

Anirudh Bagri added 5 commits March 9, 2021 11:39

updated changelog

cc0922e

black changes

3d76db8

fix test

8546730

black changes

336cc1d

fix tests

816a56d

arm4b added the enhancement label Mar 9, 2021

arm4b added this to the 3.5.0 milestone Mar 9, 2021

arm4b approved these changes Mar 9, 2021

View reviewed changes

arm4b requested a review from Kami March 9, 2021 20:06

cognifloyd approved these changes Mar 9, 2021

View reviewed changes

Merge branch 'master' into master

5f9f736

Kami reviewed Mar 12, 2021

View reviewed changes

Kami added 2 commits March 12, 2021 11:13

Update CHANGELOG.rst

49b7f9a

Merge branch 'master' into master

bedc170

Kami merged commit 514586d into StackStorm:master Mar 12, 2021

ytjohn mentioned this pull request Oct 11, 2021

Sensor raises Error when using sensor_service.list_values #5327

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add support for limit and offset in list_values method#5176

Add support for limit and offset in list_values method#5176
Kami merged 16 commits intoStackStorm:masterfrom
anirudhbagri:master

anirudhbagri commented Mar 3, 2021 •

edited by arm4b

Loading

Uh oh!

cognifloyd commented Mar 7, 2021

Uh oh!

arm4b commented Mar 8, 2021

Uh oh!

cognifloyd left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cognifloyd commented Mar 9, 2021

Uh oh!

cognifloyd commented Mar 9, 2021

Uh oh!

arm4b left a comment

Uh oh!

cognifloyd left a comment

Uh oh!

Kami Mar 12, 2021

Uh oh!

glafkosch commented Apr 15, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	# Maximum limit (page size) argument which can be specified by the user in a query string.
	max_page_size = 100

	cfg.IntOpt(
	"max_page_size",
	default=100,
	help="Maximum limit (page size) argument which can be "
	"specified by the user in a query string.",
	),

	# Maximum value of limit which can be specified by user
	@property
	def max_limit(self):
	return cfg.CONF.api.max_page_size

Uh oh!

Conversation

anirudhbagri commented Mar 3, 2021 • edited by arm4b Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cognifloyd commented Mar 7, 2021

Uh oh!

arm4b commented Mar 8, 2021

Uh oh!

cognifloyd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cognifloyd commented Mar 9, 2021

Uh oh!

cognifloyd commented Mar 9, 2021

Uh oh!

arm4b left a comment

Choose a reason for hiding this comment

Uh oh!

cognifloyd left a comment

Choose a reason for hiding this comment

Uh oh!

Kami Mar 12, 2021

Choose a reason for hiding this comment

Uh oh!

glafkosch commented Apr 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

anirudhbagri commented Mar 3, 2021 •

edited by arm4b

Loading

glafkosch commented Apr 15, 2023 •

edited

Loading