Allow pulp CLI to print sub_errors

closes #828

client_lib/pulp/client/commands/polling.py

@@ -296,6 +296,19 @@ def succeeded(self, task):
         msg = _('Task Succeeded')
         self.prompt.render_success_message(msg, tag='succeeded')
 
+    def _render_coded_error(self, e):
+        """
+        Recursively render a coded error and the errors contained by it.
+
+        :param e: error to display
+        :type  e: dictionary containing error information.
+        """
+        malformed_msg = _('Request error does not contain a description; '
+                          'please check client logs for more information.')
+        self.prompt.render_failure_message(e.get('description', malformed_msg))
+        for suberror in e.get('sub_errors', []):
+            self._render_coded_error(suberror)
+
     def failed(self, task):
         """
         Called when a task has completed with a status indicating that it failed.
@@ -306,11 +319,10 @@ def failed(self, task):
         """
         msg = _('Task Failed')
         self.prompt.render_failure_message(msg, tag='failed')
-        if task and task.exception:
+        if task and task.error:
+            self._render_coded_error(task.error)
+        elif task and task.exception:
             self.prompt.render_failure_message(task.exception, tag='failed_exception')
-        elif task and task.error and 'description' in task.error:
-            self.context.prompt.render_failure_message(task.error['description'],
-                                                       tag='error_message')
 
     def cancelled(self, task):
         """

client_lib/test/unit/client/commands/repo/test_upload.py

@@ -60,7 +60,8 @@ def test_import_upload_step_failed(self, render_failure_message):
             'task_id': '123456', 'tags': [], 'start_time': datetime.datetime.now(),
             'finish_time': datetime.datetime.now() + datetime.timedelta(seconds=10),
             'state': responses.STATE_ERROR, 'progress_report': {}, 'result': None,
-            'exception': None, 'traceback': None, 'error': 'An error message.', 'spawned_tasks': []}
+            'exception': None, 'traceback': None, 'error': {'description': 'error_message'},
+            'spawned_tasks': []}
         response = mock.MagicMock()
         response.response_body = responses.Task(response_body)
         response.is_async = mock.MagicMock(return_value=False)
@@ -73,7 +74,8 @@ def test_import_upload_step_failed(self, render_failure_message):
 
         command.perform_upload(self.context, upload_manager, upload_ids, user_input)
 
-        render_failure_message.assert_called_once_with('Task Failed', tag='failed')
+        render_failure_message.assert_has_calls([mock.call('Task Failed', tag='failed'),
+                                                 mock.call('error_message')])
 
     @mock.patch('pulp.client.extensions.core.PulpPrompt.render_failure_message')
     @mock.patch('pulp.client.extensions.core.PulpPrompt.write')

client_lib/test/unit/client/commands/test_polling.py

@@ -311,6 +311,36 @@ def test_failed_task(self):
                          ]
         self.assertEqual(set(expected_tags), set(self.prompt.get_write_tags()))
 
+    @mock.patch('pulp.client.commands.polling.PollingCommand._render_coded_error')
+    def test_failed_task_calls__render_coded_error(self, mock_render_coded):
+        """
+        When a task fails, it should call _render_coded_error to render the error and suberrors.
+        """
+        mock_task = mock.MagicMock()
+        mock_task.error = {'description': 'this is a test of the emergency broadcast system'}
+        self.command.failed(mock_task)
+        mock_render_coded.assert_called_once_with(mock_task.error)
+
+    def test__render_coded_error_called_recursively(self):
+        """
+        Pass an error with sub_errors, they should all be rendered.
+        """
+        mock_error = {'description': 'this is a test of the emergency broadcast system',
+                      'sub_errors': [{'description': 'beep1'}, {'description': 'beep2'}]}
+
+        # Though a context manager would be prefered, this pattern is necessary to run on python 2.4
+        old_prompt = self.command.prompt
+        try:
+            self.command.prompt = mock.MagicMock()
+            self.command._render_coded_error(mock_error)
+            self.command.prompt.render_failure_message.assert_has_calls([
+                mock.call('this is a test of the emergency broadcast system'),
+                mock.call('beep1'),
+                mock.call('beep2'),
+            ])
+        finally:
+            self.command.prompt = old_prompt
+
     def test_cancelled_task(self):
         """
         Task Count: 1

client_lib/test/unit/test_client_framework_core.py

@@ -223,4 +223,4 @@ def test_create_spinner(self):
 
         # Verify
         self.assertEqual(10, len(p.get_write_tags()))
-        self.assertEqual(0, len([t for t in p.get_write_tags() if t is not core.TAG_SPINNER]))
+        self.assertEqual(0, len([t for t in p.get_write_tags() if t is not core.TAG_SPINNER]))

devel/pulp/devel/mock_plugins.py

@@ -249,6 +249,10 @@ def reset():
     MOCK_PROFILER.reset_mock()
     MOCK_PROFILER_RPM.reset_mock()
 
+    # Reset does not remove side_effects
+    MOCK_DISTRIBUTOR.validate_config.side_effect = None
+    MOCK_DISTRIBUTOR_2.validate_config.side_effect = None
+
     # Undo the monkey patch
     plugin_api.get_distributor_by_id = _ORIG_GET_DISTRIBUTOR_BY_ID
     plugin_api.get_group_distributor_by_id = _ORIG_GET_GROUP_DISTRIBUTOR_BY_ID

server/pulp/server/managers/repo/distributor.py

@@ -308,22 +308,15 @@ def update_distributor_config(repo_id, distributor_id, distributor_config, auto_
                                                                          repo_id)
         config_conduit = RepoConfigConduit(distributor_type_id)
 
-        try:
-            result = distributor_instance.validate_config(transfer_repo, call_config,
+        result = distributor_instance.validate_config(transfer_repo, call_config,
                                                           config_conduit)
 
-            # For backward compatibility with plugins that don't yet return the tuple
-            if isinstance(result, bool):
-                valid_config = result
-                message = None
-            else:
-                valid_config, message = result
-        except Exception, e:
-            msg = _('Exception raised from distributor [%(d)s] while validating config for repo '
-                    '[%(r)s]')
-            msg = msg % {'d': distributor_type_id, 'r': repo_id}
-            logger.exception(msg)
-            raise PulpDataException(e.args), None, sys.exc_info()[2]
+        # For backward compatibility with plugins that don't yet return the tuple
+        if isinstance(result, bool):
+            valid_config = result
+            message = None
+        else:
+            valid_config, message = result
 
         if not valid_config:
             raise PulpDataException(message)

server/test/unit/test_repo_distributor_manager.py

@@ -240,9 +240,6 @@ def test_add_distributor_validate_raises_error(self):
         except Exception, e:
             pass
 
-        # Cleanup
-        mock_plugins.MOCK_DISTRIBUTOR.validate_config.side_effect = None
-
     def test_add_distributor_invalid_config(self):
         """
         Tests the correct error is raised when the distributor is handed an invalid configuration.
@@ -420,17 +417,13 @@ def test_update_validate_exception(self):
         distributor = self.distributor_manager.add_distributor('elf', 'mock-distributor', {}, True)
         dist_id = distributor['id']
 
-        mock_plugins.MOCK_DISTRIBUTOR.validate_config.side_effect = Exception()
-
-        # Test
-        try:
-            self.distributor_manager.update_distributor_config('elf', dist_id, {})
-            self.fail('Exception expected')
-        except exceptions.PulpDataException:
+        class TestException(Exception):
             pass
 
-        # Cleanup
-        mock_plugins.MOCK_DISTRIBUTOR.validate_config.side_effect = None
+        mock_plugins.MOCK_DISTRIBUTOR.validate_config.side_effect = TestException()
+
+        self.assertRaises(TestException, self.distributor_manager.update_distributor_config,
+                          'elf', dist_id, {})
 
     def test_update_invalid_config(self):
         """

server/test/unit/test_repo_group_distributor_manager.py

@@ -322,4 +322,4 @@ def test_find_distributors_no_group(self):
             self.distributor_manager.find_distributors('foo')
             self.fail()
         except MissingResource, e:
-            self.assertEqual(e.resources['repo_group'], 'foo')
+            self.assertEqual(e.resources['repo_group'], 'foo')