openstack/python-openstackclient
Code Issues Proposed changes

Modify error handling for role and group commands

Browse Source 
if command failed, we usually raise exception,
if command success, sometimes there is not any
output (such as set, add commands)
So modify the error handling for role and group
commands.

Change-Id: I1c0f86c04dcedd9c0d725fd73f3436be9da75ee0
This commit is contained in:
Huanxuan Ao 2017-02-09 18:05:32 +08:00
parent 46d1df0adf
commit cfd4e2a722
5 changed files with 94 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
openstackclient/identity/v3/group.py
View File

@ -62,18 +62,13 @@ class AddUserToGroup(command.Command):
try:
identity_client.users.add_to_group(user_id, group_id)
except Exception:
msg = _("%(user)s not added to group %(group)s\n") % {
except Exception as e:
msg = _("%(user)s not added to group %(group)s: %(e)s") % {
'user': parsed_args.user,
'group': parsed_args.group,
'e': e,
}
sys.stderr.write(msg)
else:
msg = _("%(user)s added to group %(group)s\n") % {
'user': parsed_args.user,
'group': parsed_args.group,
}
sys.stdout.write(msg)
raise exceptions.CommandError(msg)
class CheckUserInGroup(command.Command):
@ -306,18 +301,13 @@ class RemoveUserFromGroup(command.Command):
try:
identity_client.users.remove_from_group(user_id, group_id)
except Exception:
msg = _("%(user)s not removed from group %(group)s\n") % {
except Exception as e:
msg = _("%(user)s not removed from group %(group)s: %(e)s") % {
'user': parsed_args.user,
'group': parsed_args.group,
'e': e,
}
sys.stderr.write(msg)
else:
msg = _("%(user)s removed from group %(group)s\n") % {
'user': parsed_args.user,
'group': parsed_args.group,
}
sys.stdout.write(msg)
raise exceptions.CommandError(msg)
class SetGroup(command.Command):

29
openstackclient/identity/v3/role.py
View File

@ -16,7 +16,6 @@
"""Identity v3 Role action implementations"""
import logging
import sys
from keystoneauth1 import exceptions as ks_exc
from osc_lib.command import command
@ -129,7 +128,9 @@ class AddRole(command.Command):
if (not parsed_args.user and not parsed_args.domain
and not parsed_args.group and not parsed_args.project):
return
msg = _("Role not added, incorrect set of arguments "
"provided. See openstack --help for more details")
raise exceptions.CommandError(msg)
domain_id = None
if parsed_args.role_domain:
@ -143,11 +144,6 @@ class AddRole(command.Command):
kwargs = _process_identity_and_resource_options(
parsed_args, self.app.client_manager.identity)
if not kwargs:
sys.stderr.write(_("Role not added, incorrect set of arguments "
"provided. See openstack --help for more "
"details\n"))
return
identity_client.roles.grant(role.id, **kwargs)
@ -372,10 +368,10 @@ class ListRole(command.Lister):
'<group-name> --project <project-name> --names '
'instead.'))
else:
sys.stderr.write(_("Error: If a user or group is specified, "
"either --domain or --project must also be "
"specified to list role grants.\n"))
return ([], [])
msg = _("Error: If a user or group is specified, "
"either --domain or --project must also be "
"specified to list role grants.")
raise exceptions.CommandError(msg)
return (columns,
(utils.get_item_properties(
@ -405,9 +401,9 @@ class RemoveRole(command.Command):
if (not parsed_args.user and not parsed_args.domain
and not parsed_args.group and not parsed_args.project):
sys.stderr.write(_("Incorrect set of arguments provided. "
"See openstack --help for more details\n"))
return
msg = _("Incorrect set of arguments provided. "
"See openstack --help for more details")
raise exceptions.CommandError(msg)
domain_id = None
if parsed_args.role_domain:
@ -421,11 +417,6 @@ class RemoveRole(command.Command):
kwargs = _process_identity_and_resource_options(
parsed_args, self.app.client_manager.identity)
if not kwargs:
sys.stderr.write(_("Role not removed, incorrect set of arguments "
"provided. See openstack --help for more "
"details\n"))
return
identity_client.roles.revoke(role.id, **kwargs)

25
openstackclient/tests/functional/identity/v3/test_group.py
View File

@ -102,11 +102,7 @@ class GroupTests(common.IdentityTests):
'user_domain': self.domain_name,
'group': group_name,
'user': username})
self.assertEqual(
'%(user)s added to group %(group)s\n' % {'user': username,
'group': group_name},
raw_output
)
self.assertOutput('', raw_output)
def test_group_contains_user(self):
group_name = self._create_dummy_group()
@ -128,11 +124,7 @@ class GroupTests(common.IdentityTests):
'user_domain': self.domain_name,
'group': group_name,
'user': username})
self.assertEqual(
'%(user)s added to group %(group)s\n' % {'user': username,
'group': group_name},
raw_output
)
self.assertOutput('', raw_output)
raw_output = self.openstack(
'group contains user '
'--group-domain %(group_domain)s '
@ -165,14 +157,5 @@ class GroupTests(common.IdentityTests):
'user_domain': self.domain_name,
'group': group_name,
'user': username})
self.assertEqual(
'%(user)s added to group %(group)s\n' % {'user': username,
'group': group_name},
add_raw_output
)
self.assertEqual(
'%(user)s removed from '
'group %(group)s\n' % {'user': username,
'group': group_name},
remove_raw_output
)
self.assertOutput('', add_raw_output)
self.assertOutput('', remove_raw_output)

29
openstackclient/tests/unit/identity/v3/test_group.py
View File

@ -70,6 +70,20 @@ class TestGroupAddUser(TestGroup):
self.user.id, self.group.id)
self.assertIsNone(result)
def test_group_add_user_with_error(self):
self.users_mock.add_to_group.side_effect = exceptions.CommandError()
arglist = [
self.group.name,
self.user.name,
]
verifylist = [
('group', self.group.name),
('user', self.user.name),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
self.assertRaises(exceptions.CommandError,
self.cmd.take_action, parsed_args)
class TestGroupCheckUser(TestGroup):
@ -460,6 +474,21 @@ class TestGroupRemoveUser(TestGroup):
self.user.id, self.group.id)
self.assertIsNone(result)
def test_group_remove_user_with_error(self):
self.users_mock.remove_from_group.side_effect = (
exceptions.CommandError())
arglist = [
self.group.id,
self.user.id,
]
verifylist = [
('group', self.group.id),
('user', self.user.id),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
self.assertRaises(exceptions.CommandError,
self.cmd.take_action, parsed_args)
class TestGroupSet(TestGroup):

43
openstackclient/tests/unit/identity/v3/test_role.py
View File

@ -273,6 +273,22 @@ class TestRoleAdd(TestRole):
)
self.assertIsNone(result)
def test_role_add_with_error(self):
arglist = [
identity_fakes.role_name,
]
verifylist = [
('user', None),
('group', None),
('domain', None),
('project', None),
('role', identity_fakes.role_name),
('inherited', False),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
self.assertRaises(exceptions.CommandError,
self.cmd.take_action, parsed_args)
class TestRoleAddInherited(TestRoleAdd, TestRoleInherited):
pass
@ -771,6 +787,17 @@ class TestRoleList(TestRole):
), )
self.assertEqual(datalist, tuple(data))
def test_role_list_group_with_error(self):
arglist = [
'--group', identity_fakes.group_id,
]
verifylist = [
('group', identity_fakes.group_id),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
self.assertRaises(exceptions.CommandError,
self.cmd.take_action, parsed_args)
class TestRoleRemove(TestRole):
@ -982,6 +1009,22 @@ class TestRoleRemove(TestRole):
)
self.assertIsNone(result)
def test_role_remove_with_error(self):
arglist = [
identity_fakes.role_name,
]
verifylist = [
('user', None),
('group', None),
('domain', None),
('project', None),
('role', identity_fakes.role_name),
('inherited', False),
]
parsed_args = self.check_parser(self.cmd, arglist, verifylist)
self.assertRaises(exceptions.CommandError,
self.cmd.take_action, parsed_args)
class TestRoleSet(TestRole):