Merge "Use CommandError, not SystemExit, to exit"

Zuul · openstack-gerrit · commit 2d32d87c16c5 · 2023-12-05T18:35:47.000Z
diff --git a/openstackclient/compute/v2/server.py b/openstackclient/compute/v2/server.py
@@ -1987,9 +1987,10 @@ def _match_image(image_api, wanted_properties):
             ):
                 self.app.stdout.write('\n')
             else:
-                LOG.error('Error creating server: %s', parsed_args.server_name)
-                self.app.stdout.write(_('Error creating server\n'))
-                raise SystemExit
+                msg = (
+                    _('Error unshelving server: %s') % parsed_args.server_name
+                )
+                raise exceptions.CommandError(msg)
 
         details = _prep_server_detail(compute_client, image_client, server)
         return zip(*sorted(details.items()))
@@ -2081,10 +2082,8 @@ def _show_progress(progress):
                     server_obj.id,
                     callback=_show_progress,
                 ):
-                    msg = _('Error deleting server: %s')
-                    LOG.error(msg, server_obj.id)
-                    self.app.stdout.write(_('Error deleting server\n'))
-                    raise SystemExit
+                    msg = _('Error deleting server: %s') % server_obj.id
+                    raise exceptions.CommandError(msg)
 
 
 def percent_type(x):
@@ -3110,9 +3109,8 @@ def _show_progress(progress):
             ):
                 self.app.stdout.write(_('Complete\n'))
             else:
-                LOG.error(_('Error migrating server: %s'), server.id)
-                self.app.stdout.write(_('Error migrating server\n'))
-                raise SystemExit
+                msg = _('Error migrating server: %s') % server.id
+                raise exceptions.CommandError(msg)
 
 
 class PauseServer(command.Command):
@@ -3194,9 +3192,8 @@ def _show_progress(progress):
             ):
                 self.app.stdout.write(_('Complete\n'))
             else:
-                LOG.error(_('Error rebooting server: %s'), server_id)
-                self.app.stdout.write(_('Error rebooting server\n'))
-                raise SystemExit
+                msg = _('Error rebooting server: %s') % server_id
+                raise exceptions.CommandError(msg)
 
 
 class RebuildServer(command.ShowOne):
@@ -3566,9 +3563,8 @@ def _show_progress(progress):
             ):
                 self.app.stdout.write(_('Complete\n'))
             else:
-                LOG.error(_('Error rebuilding server: %s'), server.id)
-                self.app.stdout.write(_('Error rebuilding server\n'))
-                raise SystemExit
+                msg = _('Error rebuilding server: %s') % server.id
+                raise exceptions.CommandError(msg)
 
         details = _prep_server_detail(
             compute_client, image_client, server, refresh=False
@@ -3689,9 +3685,8 @@ def _show_progress(progress):
             ):
                 self.app.stdout.write(_('Complete\n'))
             else:
-                LOG.error(_('Error evacuating server: %s'), server.id)
-                self.app.stdout.write(_('Error evacuating server\n'))
-                raise SystemExit
+                msg = _('Error evacuating server: %s') % server.id
+                raise exceptions.CommandError(msg)
 
         details = _prep_server_detail(
             compute_client, image_client, server, refresh=True
@@ -4042,9 +4037,8 @@ def _show_progress(progress):
                 ):
                     self.app.stdout.write(_('Complete\n'))
                 else:
-                    LOG.error(_('Error resizing server: %s'), server.id)
-                    self.app.stdout.write(_('Error resizing server\n'))
-                    raise SystemExit
+                    msg = _('Error resizing server: %s') % server.id
+                    raise exceptions.CommandError(msg)
         elif parsed_args.confirm:
             self.log.warning(
                 _(
@@ -4445,11 +4439,8 @@ def _show_progress(progress):
                 success_status=('shelved', 'shelved_offloaded'),
                 callback=_show_progress,
             ):
-                LOG.error(_('Error shelving server: %s'), server_id)
-                self.app.stdout.write(
-                    _('Error shelving server: %s\n') % server_id
-                )
-                raise SystemExit
+                msg = _('Error shelving server: %s') % server_id
+                raise exceptions.CommandError(msg)
 
         if not parsed_args.offload:
             return
@@ -4474,14 +4465,8 @@ def _show_progress(progress):
                 success_status=('shelved_offloaded',),
                 callback=_show_progress,
             ):
-                LOG.error(
-                    _('Error offloading shelved server %s'),
-                    server_id,
-                )
-                self.app.stdout.write(
-                    _('Error offloading shelved server: %s\n') % server_id
-                )
-                raise SystemExit
+                msg = _('Error offloading shelved server: %s') % server_id
+                raise exceptions.CommandError(msg)
 
 
 class ShowServer(command.ShowOne):
@@ -5073,8 +5058,5 @@ def _show_progress(progress):
                     success_status=('active', 'shutoff'),
                     callback=_show_progress,
                 ):
-                    LOG.error(_('Error unshelving server %s'), server_obj.id)
-                    self.app.stdout.write(
-                        _('Error unshelving server: %s\n') % server_obj.id
-                    )
-                    raise SystemExit
+                    msg = _('Error unshelving server: %s') % server_obj.id
+                    raise exceptions.CommandError(msg)
diff --git a/openstackclient/tests/unit/compute/v2/test_server.py b/openstackclient/tests/unit/compute/v2/test_server.py
@@ -2327,7 +2327,9 @@ def test_server_create_with_wait_fails(self, mock_wait_for_status):
 
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-        self.assertRaises(SystemExit, self.cmd.take_action, parsed_args)
+        self.assertRaises(
+            exceptions.CommandError, self.cmd.take_action, parsed_args
+        )
 
         mock_wait_for_status.assert_called_once_with(
             self.servers_mock.get,
@@ -4569,7 +4571,9 @@ def test_server_delete_wait_fails(self, mock_wait_for_delete):
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-        self.assertRaises(SystemExit, self.cmd.take_action, parsed_args)
+        self.assertRaises(
+            exceptions.CommandError, self.cmd.take_action, parsed_args
+        )
 
         self.servers_mock.delete.assert_called_with(servers[0].id)
         mock_wait_for_delete.assert_called_once_with(
@@ -5984,7 +5988,9 @@ def test_server_migrate_with_wait_fails(self, mock_wait_for_status):
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-        self.assertRaises(SystemExit, self.cmd.take_action, parsed_args)
+        self.assertRaises(
+            exceptions.CommandError, self.cmd.take_action, parsed_args
+        )
 
         self.servers_mock.get.assert_called_with(self.server.id)
         self.server.migrate.assert_called_with()
@@ -6090,9 +6096,10 @@ def test_server_reboot_with_wait_fails(
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-        self.assertRaises(SystemExit, self.cmd.take_action, parsed_args)
+        self.assertRaises(
+            exceptions.CommandError, self.cmd.take_action, parsed_args
+        )
 
-        self.assertIn('Error rebooting server', mock_log.call_args[0][0])
         self.compute_sdk_client.reboot_server.assert_called_once_with(
             servers[0].id,
             'SOFT',
@@ -6351,7 +6358,9 @@ def test_rebuild_with_wait_fails(self, mock_wait_for_status):
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-        self.assertRaises(SystemExit, self.cmd.take_action, parsed_args)
+        self.assertRaises(
+            exceptions.CommandError, self.cmd.take_action, parsed_args
+        )
 
         mock_wait_for_status.assert_called_once_with(
             self.servers_mock.get,
@@ -7519,7 +7528,9 @@ def test_server_resize_with_wait_fails(self, mock_wait_for_status):
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
-        self.assertRaises(SystemExit, self.cmd.take_action, parsed_args)
+        self.assertRaises(
+            exceptions.CommandError, self.cmd.take_action, parsed_args
+        )
 
         self.servers_mock.get.assert_called_with(
             self.server.id,
