Skip to content

Commit

Permalink
Fixing wok_log redundancy and i18n.py error messages
Browse files Browse the repository at this point in the history 
This patch removes wok_log calls when they are just
reproducing the same message of a call to a wok.exception
(NotFoundError, OperationFailed ...) class. Wok exceptions
already write the messages into the log and calling
wok_log is redundant.

Any other use of wok_log was preserved (logging a message
that didn't appear to the user, for example).

i18n.py messages were fixed along the way (wrong use of
message strings, typos, etc).

Affected files: model/swaps.py, model/users.py,
model/utils.py, model/vol_group.py

Signed-off-by: Daniel Henrique Barboza <[email protected]>
  • Loading branch information
@danielhb
danielhb committed Jul 14, 2016
1 parent 6811742 commit 4cb9b33
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 282 deletions.
37 changes: 21 additions & 16 deletions i18n.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,12 @@
"GINUSER0006E": _("Could not add user '%(user)s' to kvm group."),
"GINUSER0007E": _("Could not add user '%(user)s' to sudoers list, error: '%(err)s'."),
"GINUSER0008E": _("User/Login '%(user)s' already in use."),
"GINUSER0009E": _("There was a problem while creating user '%(user)s', error: '%(err)s'."),
"GINUSER0010E": _("There was a problem while deleting user '%(user)s', error: '%(err)s'."),
"GINUSER0009E": _("There was a problem while creating user '%(user)s', error: user name is empty."),
"GINUSER0010E": _("There was a problem while deleting user '%(user)s', error: user name is empty."),
"GINUSER0011E": _("User '%(user)s' does not exist."),
"GINUSER0012E": _("There was a problem while deleting group '%(group)s', error: '%(err)s'."),
"GINUSER0012E": _("There was a problem while deleting group '%(group)s'. Group name is an empty string."),
"GINUSER0013E": _("Failed to remove user '%(user)s' from sudoers list"),
"GINUSER0014E": _("There was a problem while creating group '%(group)s', error: %(err)s'."),
"GINUSER0014E": _("There was a problem while creating group '%(group)s'. Group name is an empty string."),
"GINUSER0015E": _("User 'no_login' should be boolean."),
'GINUSER0016E': _("Deletion is not supported for users found in '/etc/sudoers' file. User: %(user)s"),
'GINUSER0017E': _("Deletion is not supported for group found in '/etc/sudoers' file. Group: %(group)s"),
Expand All @@ -137,6 +137,9 @@
"GINUSER0022E": _("There was a problem while reading users details, see log for details"),
"GINUSER0023E": _("There was a problem while fetching user profile, see log for details"),
"GINUSER0024E": _("Failed to change password for user '%(user)s', error: '%(err)s'."),
"GINUSER0025E": _("There was a problem while creating user '%(user)s', error: group id %(gid)s is not of integer type."),
"GINUSER0026E": _("There was a problem while creating user '%(user)s', error: no_login id %(no_login)s is not of boolean type."),
"GINUSER0027E": _("Failed to remove user '%(user)s' from group '%(group)s'. Error: group name is an empty string."),

"GINFW0001E": _("Cannot update system firmware while running VMs."),
"GINFW0002E": _("Firmware image unpack failed: rc = %(rc)s. "
Expand Down Expand Up @@ -212,21 +215,22 @@
"GINSP00003E": _("Size is mandatory while creating file type swap device."),
"GINSP00004E": _("Incorrect swap type, only 'device' and 'file' are allowed."),
"GINSP00005E": _("Error creating a swap device. %(err)s "),
"GINSP00006E": _("Error deleting a swap file."),
"GINSP00006E": _("Error deleting a swap file: %(err)s"),
"GINSP00007E": _("Error deleting a swap device. %(err)s"),
"GINSP00008E": _("Swap device not found. %(name)s"),
"GINSP00009E": _("Error deleting a swap file. %(err)s"),
"GINSP00009E": _("Error deleting a swap device. %(err)s"),
"GINSP00010E": _("Error parsing /proc/swaps file. %(err)s"),
"GINSP00011E": _("Error creating a flat file. %(err)s"),
"GINSP00012E": _("Unable to format swap device. %(err)s"),
"GINSP00013E": _("Unable to activate swap device. %(err)s"),
"GINSP00014E": _("Unable to parse 'swapon -s' output. %(err)s"),
"GINSP00015E": _("Unable to get single swap device info. %(err)s"),
"GINSP00016E": _("Unable to activate swap device. %(err)s"),
"GINSP00017E": _("No partitions found for disk . %(disk)s"),
"GINSP00016E": _("Unable to deactivate swap device. %(err)s"),
"GINSP00017E": _("No partitions found for disk %(disk)s"),
"GINSP00018E": _("Single swap device %(swap)s not found."),
"GINSP00019E": _("Unable to get single swap device info: directory /proc/swaps not found."),
"GINSP00020E": _("File already in use."),
"GINSP00020E": _("File %(file)s already in use."),
"GINSP00021E": _("Unable to change the partition type. Error: %(err)s"),

"GINPART00001E": _("Fetching list of partitions failed. Error: %(err)s"),
"GINPART00002E": _("Create partition failed. Error: %(err)s"),
Expand All @@ -242,7 +246,7 @@
"GINPART00013E": _("No partitions found"),
"GINPART00014E": _("Partition %(name)s not found."),

"GINLVM0001E": _("Error parsing output of 'lvm version' command.. %(err)s"),
"GINLVM0001E": _("Error parsing output of 'lvm version' command: %(err)s"),
"GINLVM0002E": _("Error executing 'lvm version' command. %(err)s"),
"GINLVM0003E": _("Incompatible output from 'lvm version' command. %(err)s"),

Expand All @@ -258,12 +262,12 @@
"GINPV00010E": _("Remove failed: device %(dev)s not found."),
"GINPV00011E": _("Unable to find device %(dev)s ."),

"GINVG00001E": _("Failed to create VG"),
"GINVG00002E": _("Failed to list VGs"),
"GINVG00003E": _("Failed to fetch VG details"),
"GINVG00004E": _("Failed to delete VG"),
"GINVG00005E": _("Failed to extend VG"),
"GINVG00006E": _("Failed to reduce VG"),
"GINVG00001E": _("Failed to create VG %(name)s. Error: %(err)s"),
"GINVG00002E": _("Failed to list VGs. Error: %(err)s"),
"GINVG00003E": _("Failed to fetch VG %(name)s details."),
"GINVG00004E": _("Failed to delete VG %(name)s."),
"GINVG00005E": _("Failed to extend VG %(name)s."),
"GINVG00006E": _("Failed to reduce VG %(name)s."),
"GINVG00007E": _("vgs command failed"),
"GINVG00008E": _("vgs command for given VG failed"),
"GINVG00009E": _("vgcreate command failed"),
Expand Down Expand Up @@ -301,6 +305,7 @@
"GINDASD0012E": _("Error in executing 'lscss -d' command : %(err)s"),
"GINDASD0013E": _("Error in parsing 'lscss -d' command : %(err)s"),
"GINDASD0014E": _("No more than %(max_dasd_fmt)s concurrent DASD format operations are permitted."),
"GINDASD0015E": _("Header is empty for given pattern."),

"GINDASDPAR0005E": _("Require name to create DASD device partition"),
"GINDASDPAR0006E": _("Require partition size to create DASD device partition"),
Expand Down
10 changes: 2 additions & 8 deletions model/swaps.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from wok.exception import InvalidParameter, NotFoundError, OperationFailed
from wok.model.tasks import TaskModel
from wok.rollbackcontext import RollbackContext
from wok.utils import add_task, run_command, wok_log
from wok.utils import add_task, run_command


class SwapsModel(object):
Expand All @@ -44,23 +44,19 @@ def create(self, params):
file_loc = ''

if 'file_loc' not in params or not params['file_loc']:
wok_log.error("File location required for creating a swap device.")
raise InvalidParameter('GINSP00001E')

if 'type' not in params:
wok_log.error("Type required for creating a swap device.")
raise InvalidParameter('GINSP00002E')
else:
if params['type'] == 'file' and 'size' not in params:
wok_log.error("Size is required file type swap device.")
raise InvalidParameter('GINSP00003E')

if params['type'] == 'device' or params['type'] == 'file':
taskid = add_task(u'/swaps/file_loc/%s' % (file_loc),
self._create_task, self.objstore, params)
return self.task.lookup(taskid)
else:
wok_log.error("Incorrect swap type.")
raise InvalidParameter('GINSP00004E')

def _create_task(self, cb, params):
Expand Down Expand Up @@ -131,8 +127,7 @@ def delete_swap_file(file_loc):
os.remove(file_loc)

except Exception as e:
wok_log.error("Error deleting a swap file, %s", e.message)
raise OperationFailed('GINSP00006E')
raise OperationFailed('GINSP00006E', {'err': e.message})

def get_list(self):
out, err, rc = run_command(["cat", "/proc/swaps"])
Expand Down Expand Up @@ -166,5 +161,4 @@ def delete(self, name):
else:
utils._swapoff_device(name)
except Exception as e:
wok_log.error("Error deleting a swap device, %s", e.message)
raise OperationFailed("GINSP00009E", {'err': e.message})
70 changes: 12 additions & 58 deletions model/users.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,8 @@ def create_group(groupname):
:param groupname: non-empty string
:return: group id(gid)
"""
wok_log.info('in create_group() method. group name: %s' % groupname)
if not isinstance(groupname, str) or not groupname.strip():
wok_log.error('group name is not non-empty string. '
'group name %s' % groupname)
raise InvalidParameter('GINUSER0014E',
{'group': groupname,
'err': 'see log for details'})
raise InvalidParameter('GINUSER0014E', {'group': groupname})
adm = libuser.admin()
if adm.lookupGroupByName(groupname):
raise OperationFailed('GINUSER0018E', {'group': groupname})
Expand All @@ -83,21 +78,16 @@ def create_group(groupname):
if new_group[libuser.GIDNUMBER][0] < 1000:
new_group.set(libuser.GIDNUMBER, adm.getFirstUnusedGid(1000))
adm.addGroup(new_group)
wok_log.info('successfully created group. group name: %s.' % groupname)
return new_group[libuser.GIDNUMBER][0]

except Exception as e:
raise OperationFailed('GINUSER0014E', {'group': groupname, 'err': e})


def delete_group(groupname):
wok_log.info('in delete_group(). group name: %s' % groupname)
if not isinstance(groupname, str) or not groupname.strip():
wok_log.error('group name is not non-empty string. '
'group name %s' % groupname)
raise InvalidParameter('GINUSER0012E',
{'group': groupname,
'err': 'see log for details'})
{'group': groupname})
adm = libuser.admin()
group_id = int(get_group_gid(groupname))

Expand Down Expand Up @@ -167,21 +157,13 @@ def create_user(name, plain_passwd, gid, no_login=False):
:param gid: primary group id for user
:param no_login: True/False for log in shell
"""
wok_log.info('in create_user() method')
if not isinstance(name, str) or not name.strip():
wok_log.error('user name is not non-empty string. name: %s' % name)
raise InvalidParameter(
'GINUSER0009E', {'user': name, 'err': 'see log for details'})
raise InvalidParameter('GINUSER0009E', {'user': name})
if not type(gid) in [int, long]:
wok_log.error('group id is not of integer type. gid: %s' % gid)
raise InvalidParameter('GINUSER0009E', {'user': name,
'err': 'see log for details'})
raise InvalidParameter('GINUSER0025E', {'user': name, 'gid': gid})
if not isinstance(no_login, bool):
wok_log.error('no_login id is not of boolean type.'
' no_login: %s' % no_login)
raise InvalidParameter('GINUSER0009E',
{'user': name,
'err': 'see log for details'})
raise InvalidParameter('GINUSER0026E', {'user': name,
'no_login': no_login})
adm = libuser.admin()
if adm.lookupUserByName(name):
raise OperationFailed('GINUSER0008E', {'user': name})
Expand Down Expand Up @@ -229,11 +211,8 @@ def delete_user(username):
method to delete user
:param username: user name
"""
wok_log.info('in delete_user() method')
if not isinstance(username, str) or not username.strip():
wok_log.error('username is not non-empty string. name: %s' % username)
raise InvalidParameter('GINUSER0010E', {'user': username,
'err': 'see log for details'})
raise InvalidParameter('GINUSER0010E', {'user': username})
if username in get_sudoers(admin_check=False):
raise OperationFailed('GINUSER0016E', {'user': username})

Expand All @@ -253,7 +232,6 @@ def delete_user(username):
try:
os.unlink(f)
except Exception as e:
wok_log.error('Error removing file "%s": %s' % (f, e))
raise OperationFailed('GINUSER0013E', {'user': username})
try:
adm.deleteUser(user_obj, True, True)
Expand All @@ -268,16 +246,11 @@ def remove_user_from_group(user, group):
:param user: user name
:param group: group name
"""
wok_log.info('in remove_user_from_group() method')
if not isinstance(user, str) or not user.strip():
wok_log.error('user name is not non-empty string. name: %s' % user)
raise InvalidParameter('GINUSER0010E',
{'user': user, 'err': 'see log for details'})
raise InvalidParameter('GINUSER0010E', {'user': user})
if not isinstance(group, str) or not group.strip():
wok_log.error('group name is not non-empty string. '
'group name %s' % group)
raise InvalidParameter('GINUSER0010E',
{'user': user, 'err': 'see log for details'})
raise InvalidParameter('GINUSER0027E',
{'user': user, 'group': group})
try:
adm = libuser.admin()
grpobj = adm.lookupGroupByName(group)
Expand All @@ -300,7 +273,6 @@ def get_sudoers(admin_check=True):
:param admin_check: True/False (to check admin/just part of sudoers file)
:return: list of users and groups
"""
wok_log.info('in get_sudoers() method')
sudoers = []
try:
parser = augeas.Augeas()
Expand Down Expand Up @@ -340,7 +312,6 @@ def get_users_info(users):
:param users: list of users
:return: list of dictionary with user details
"""
wok_log.info('in get_users_info(). Users: %s' % users)
if not isinstance(users, list):
wok_log.error('Users is not of type list. Users: %s' % users)
raise InvalidParameter('GINUSER0022E')
Expand Down Expand Up @@ -381,7 +352,6 @@ def get_user_profile(user, sudoers=None):
:return: admin, virtuser or regularuser
"""
if not isinstance(user, str) or not user.strip():
wok_log.error('username is not non-empty string. name: %s' % user)
raise InvalidParameter('GINUSER0002E')
if sudoers and not isinstance(sudoers, list):
wok_log.error('Sudoers is not of type list. Sudoers: %s' % sudoers)
Expand Down Expand Up @@ -452,10 +422,7 @@ def _validate_create_params(self, params):
:return: params dictionary with keys: name, password,
profile, group and no_login
"""
wok_log.info('in _validate_create_params(). params: %s' % params)
if not isinstance(params, dict):
wok_log.error(
'Params is not of type dictionary. Params: %s' % params)
raise InvalidParameter('GINUSER0001E')
keys = params.keys()
if isinstance(params['name'], unicode):
Expand Down Expand Up @@ -496,17 +463,12 @@ def _validate_create_params(self, params):
params['profile'] = 'regularuser'
params['no_login'] = False

wok_log.info(
'End of _validate_create_params(). Return params: %s' % params)
return params

def _add_user_to_kvm_group(self, user):
# Add new user to KVM group
if not isinstance(user, str) or not user.strip():
wok_log.error('username is not non-empty string. name: %s' % user)
raise InvalidParameter('GINUSER0009E',
{'user': user,
'err': 'see log for details'})
raise InvalidParameter('GINUSER0009E', {'user': user})
adm = libuser.admin()
kvmgrp = get_group_obj('kvm')
if isinstance(user, unicode):
Expand All @@ -518,10 +480,7 @@ def _add_user_to_kvm_group(self, user):

def _add_user_to_sudoers(self, user):
if not isinstance(user, str) or not user.strip():
wok_log.error('username is not non-empty string. name: %s' % user)
raise InvalidParameter('GINUSER0009E',
{'user': user,
'err': 'see log for details'})
raise InvalidParameter('GINUSER0009E', {'user': user})
try:
# Creates the file in /etc/sudoers.d with proper user permission
with open(SUDOERS_FILE % user, 'w') as f:
Expand Down Expand Up @@ -570,7 +529,6 @@ def lookup(self, user):
return user_info[0]

def chpasswd(self, user, password):
wok_log.info("In UserModel.chpasswd method")
if isinstance(user, unicode):
user = user.encode('utf-8')
if not isinstance(user, str) or not user.strip():
Expand All @@ -582,16 +540,12 @@ def chpasswd(self, user, password):
adm = libuser.admin()
user_obj = adm.lookupUserByName(user)
if not user_obj:
wok_log.error("User '%s' not found" % user)
raise NotFoundError('GINUSER0011E', {'user': user})
try:
adm.setpassUser(user_obj, password, False)
# False flag indicates that password is not encrypted
wok_log.info("Successfully changed password for user '%s'" % user)
except Exception as e:
err_msg = e.message if e.message else e
wok_log.error("Failed to change password for user '%s'. "
"Error: '%s'" % (user, err_msg))
raise OperationFailed(
'GINUSER0024E', {'user': user, 'err': err_msg})
wok_log.info("End of UserModel.chpasswd method")
Loading

0 comments on commit 4cb9b33

Please sign in to comment.