Set the default value of --limit parameter
The "--limit" parameter of heavy CLI commands should not be infinite by default. For commands like "mistral task-list" and "execution-list" we need to set --limit parameter by default to some reasonable value. The proposal is 100. Otherwise it's easy to cause a huge load on a server if the result set is not limited. A warning message is thrown whenever these commands are executed without using "--limit" parameter. Change-Id: If0fead181171e5b9c590f3f4c852c6ad2541ff94 Implements: blueprint mistral-cli-default-limit-parameter
This commit is contained in:
		@@ -63,12 +63,17 @@ class ActionExecutionManager(base.ResourceManager):
 | 
			
		||||
 | 
			
		||||
        return self._update('/action_executions/%s' % id, data)
 | 
			
		||||
 | 
			
		||||
    def list(self, task_execution_id=None):
 | 
			
		||||
    def list(self, task_execution_id=None, limit=None):
 | 
			
		||||
        url = '/action_executions'
 | 
			
		||||
 | 
			
		||||
        qparams = {}
 | 
			
		||||
 | 
			
		||||
        if task_execution_id:
 | 
			
		||||
            url = '/tasks/%s/action_executions' % task_execution_id
 | 
			
		||||
 | 
			
		||||
        if limit:
 | 
			
		||||
            qparams['limit'] = limit
 | 
			
		||||
 | 
			
		||||
        return self._list(url, response_key='action_executions')
 | 
			
		||||
 | 
			
		||||
    def get(self, id):
 | 
			
		||||
 
 | 
			
		||||
@@ -182,14 +182,29 @@ class List(base.MistralLister):
 | 
			
		||||
            nargs='?',
 | 
			
		||||
            help='Task execution ID.'
 | 
			
		||||
        )
 | 
			
		||||
        parser.add_argument(
 | 
			
		||||
            '--limit',
 | 
			
		||||
            type=int,
 | 
			
		||||
            help='Maximum number of action-executions to return in a single '
 | 
			
		||||
                 'result. limit is set to %s by default. Use --limit -1 to '
 | 
			
		||||
                 'fetch the full result set.' % base.DEFAULT_LIMIT,
 | 
			
		||||
            nargs='?'
 | 
			
		||||
        )
 | 
			
		||||
 | 
			
		||||
        return parser
 | 
			
		||||
 | 
			
		||||
    def _get_resources(self, parsed_args):
 | 
			
		||||
        if parsed_args.limit is None:
 | 
			
		||||
            parsed_args.limit = base.DEFAULT_LIMIT
 | 
			
		||||
            LOG.info("limit is set to %s by default. Set "
 | 
			
		||||
                     "the limit explicitly using \'--limit\', if required. "
 | 
			
		||||
                     "Use \'--limit\' -1 to fetch the full result set.",
 | 
			
		||||
                     base.DEFAULT_LIMIT)
 | 
			
		||||
        mistral_client = self.app.client_manager.workflow_engine
 | 
			
		||||
 | 
			
		||||
        return mistral_client.action_executions.list(
 | 
			
		||||
            parsed_args.task_execution_id
 | 
			
		||||
            parsed_args.task_execution_id,
 | 
			
		||||
            limit=parsed_args.limit,
 | 
			
		||||
        )
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -21,6 +21,9 @@ from osc_lib.command import command
 | 
			
		||||
import six
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
DEFAULT_LIMIT = 100
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
@six.add_metaclass(abc.ABCMeta)
 | 
			
		||||
class MistralLister(command.Lister):
 | 
			
		||||
    @abc.abstractmethod
 | 
			
		||||
 
 | 
			
		||||
@@ -93,7 +93,9 @@ class List(base.MistralLister):
 | 
			
		||||
        parser.add_argument(
 | 
			
		||||
            '--limit',
 | 
			
		||||
            type=int,
 | 
			
		||||
            help='Maximum number of executions to return in a single result.',
 | 
			
		||||
            help='Maximum number of executions to return in a single result. '
 | 
			
		||||
                 'limit is set to %s by default. Use --limit -1 to fetch the '
 | 
			
		||||
                 'full result set.' % base.DEFAULT_LIMIT,
 | 
			
		||||
            nargs='?'
 | 
			
		||||
        )
 | 
			
		||||
        parser.add_argument(
 | 
			
		||||
@@ -122,6 +124,12 @@ class List(base.MistralLister):
 | 
			
		||||
        return parser
 | 
			
		||||
 | 
			
		||||
    def _get_resources(self, parsed_args):
 | 
			
		||||
        if parsed_args.limit is None:
 | 
			
		||||
            parsed_args.limit = base.DEFAULT_LIMIT
 | 
			
		||||
            LOG.info("limit is set to %s by default. Set "
 | 
			
		||||
                     "the limit explicitly using \'--limit\', if required. "
 | 
			
		||||
                     "Use \'--limit\' -1 to fetch the full result set.",
 | 
			
		||||
                     base.DEFAULT_LIMIT)
 | 
			
		||||
        mistral_client = self.app.client_manager.workflow_engine
 | 
			
		||||
 | 
			
		||||
        return mistral_client.executions.list(
 | 
			
		||||
 
 | 
			
		||||
@@ -80,6 +80,14 @@ class List(base.MistralLister):
 | 
			
		||||
            action='append',
 | 
			
		||||
            help='Filters. Can be repeated.'
 | 
			
		||||
        )
 | 
			
		||||
        parser.add_argument(
 | 
			
		||||
            '--limit',
 | 
			
		||||
            type=int,
 | 
			
		||||
            help='Maximum number of tasks to return in a single result. '
 | 
			
		||||
                 'limit is set to %s by default. Use --limit -1 to fetch the '
 | 
			
		||||
                 'full result set.' % base.DEFAULT_LIMIT,
 | 
			
		||||
            nargs='?'
 | 
			
		||||
        )
 | 
			
		||||
 | 
			
		||||
        return parser
 | 
			
		||||
 | 
			
		||||
@@ -87,10 +95,17 @@ class List(base.MistralLister):
 | 
			
		||||
        return format_list
 | 
			
		||||
 | 
			
		||||
    def _get_resources(self, parsed_args):
 | 
			
		||||
        if parsed_args.limit is None:
 | 
			
		||||
            parsed_args.limit = base.DEFAULT_LIMIT
 | 
			
		||||
            LOG.info("limit is set to %s by default. Set "
 | 
			
		||||
                     "the limit explicitly using \'--limit\', if required. "
 | 
			
		||||
                     "Use \'--limit\' -1 to fetch the full result set.",
 | 
			
		||||
                     base.DEFAULT_LIMIT)
 | 
			
		||||
        mistral_client = self.app.client_manager.workflow_engine
 | 
			
		||||
 | 
			
		||||
        return mistral_client.tasks.list(
 | 
			
		||||
            parsed_args.workflow_execution,
 | 
			
		||||
            limit=parsed_args.limit,
 | 
			
		||||
            **base.get_filters(parsed_args)
 | 
			
		||||
        )
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
@@ -97,6 +97,16 @@ class SimpleMistralCLITests(base.MistralCLIAuth):
 | 
			
		||||
            ['ID', 'Name', 'Workflow name', 'State', 'Accepted']
 | 
			
		||||
        )
 | 
			
		||||
 | 
			
		||||
    def test_action_execution_list_with_limit(self):
 | 
			
		||||
        act_execs = self.parser.listing(
 | 
			
		||||
            self.mistral(
 | 
			
		||||
                'action-execution-list',
 | 
			
		||||
                params='--limit 1'
 | 
			
		||||
            )
 | 
			
		||||
        )
 | 
			
		||||
 | 
			
		||||
        self.assertEqual(1, len(act_execs))
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
class WorkbookCLITests(base_v2.MistralClientTestBase):
 | 
			
		||||
    """Test suite checks commands to work with workbooks."""
 | 
			
		||||
@@ -853,6 +863,26 @@ class TaskCLITests(base_v2.MistralClientTestBase):
 | 
			
		||||
 | 
			
		||||
        self.assertEqual('goodbye', tasks[0]['Name'])
 | 
			
		||||
 | 
			
		||||
    def test_task_list_with_limit(self):
 | 
			
		||||
        wf_exec = self.execution_create(
 | 
			
		||||
            "%s input task_name" % self.reverse_wf['Name']
 | 
			
		||||
        )
 | 
			
		||||
 | 
			
		||||
        exec_id = self.get_field_value(wf_exec, 'ID')
 | 
			
		||||
 | 
			
		||||
        self.assertTrue(self.wait_execution_success(exec_id))
 | 
			
		||||
 | 
			
		||||
        tasks = self.parser.listing(self.mistral('task-list'))
 | 
			
		||||
 | 
			
		||||
        tasks = self.parser.listing(
 | 
			
		||||
            self.mistral(
 | 
			
		||||
                'task-list',
 | 
			
		||||
                params='--limit 1'
 | 
			
		||||
            )
 | 
			
		||||
        )
 | 
			
		||||
 | 
			
		||||
        self.assertEqual(1, len(tasks))
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
class ActionCLITests(base_v2.MistralClientTestBase):
 | 
			
		||||
    """Test suite checks commands to work with actions."""
 | 
			
		||||
 
 | 
			
		||||
@@ -229,7 +229,7 @@ class TestCLIExecutionsV2(base.BaseCommandTest):
 | 
			
		||||
 | 
			
		||||
        self.call(execution_cmd.List)
 | 
			
		||||
        self.client.executions.list.assert_called_once_with(
 | 
			
		||||
            limit=None,
 | 
			
		||||
            limit=100,
 | 
			
		||||
            marker='',
 | 
			
		||||
            sort_dirs='asc',
 | 
			
		||||
            sort_keys='created_at',
 | 
			
		||||
 
 | 
			
		||||
@@ -0,0 +1,7 @@
 | 
			
		||||
---
 | 
			
		||||
critical:
 | 
			
		||||
  - |
 | 
			
		||||
    The "--limit" parameter of heavy CLI commands like "task-list" and
 | 
			
		||||
    "execution-list" and "action-execution-list" is set to "100" by
 | 
			
		||||
    default to avoid the huge load on server. To fetch the full result
 | 
			
		||||
    set, user may use "--limit -1".
 | 
			
		||||
		Reference in New Issue
	
	Block a user