Mercurial > kallithea
changeset 4746:cc1ab5ef6686
cleanup: avoid some 'except Exception' catching - catch specific exceptions or log it and show what happened
This has a risk of introducing regressions ... but we want to get rid of all
exception muting and make the whole system less fragile and easier to debug.
line wrap: on
line diff
--- a/kallithea/bin/kallithea_api.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/bin/kallithea_api.py Tue Jan 06 00:54:36 2015 +0100 @@ -101,7 +101,7 @@ try: margs = dict(map(lambda s: s.split(':', 1), other)) - except Exception: + except ValueError: sys.stderr.write('Error parsing arguments \n') sys.exit() if args.format == FORMAT_PRETTY:
--- a/kallithea/bin/kallithea_config.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/bin/kallithea_config.py Tue Jan 06 00:54:36 2015 +0100 @@ -152,10 +152,7 @@ if argv is None: argv = sys.argv - try: - return _run(argv) - except Exception: - raise + return _run(argv) if __name__ == '__main__':
--- a/kallithea/controllers/admin/admin.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/controllers/admin/admin.py Tue Jan 06 00:54:36 2015 +0100 @@ -131,11 +131,7 @@ #FILTERING c.search_term = request.GET.get('filter') - try: - users_log = _journal_filter(users_log, c.search_term) - except Exception: - # we want this to crash for now - raise + users_log = _journal_filter(users_log, c.search_term) users_log = users_log.order_by(UserLog.action_date.desc())
--- a/kallithea/controllers/changeset.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/controllers/changeset.py Tue Jan 06 00:54:36 2015 +0100 @@ -75,10 +75,7 @@ ig_ws_global = GET.get('ignorews') ig_ws = filter(lambda k: k.startswith('WS'), GET.getall(fid)) if ig_ws: - try: - return int(ig_ws[0].split(':')[-1]) - except Exception: - pass + return int(ig_ws[0].split(':')[-1]) return ig_ws_global
--- a/kallithea/controllers/error.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/controllers/error.py Tue Jan 06 00:54:36 2015 +0100 @@ -89,7 +89,7 @@ [400, 401, 403, 404, 500]""" try: code = int(code) - except Exception: + except ValueError: code = 500 if code == 400:
--- a/kallithea/controllers/journal.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/controllers/journal.py Tue Jan 06 00:54:36 2015 +0100 @@ -27,6 +27,7 @@ """ import logging +import traceback from itertools import groupby from sqlalchemy import or_ @@ -95,11 +96,7 @@ .options(joinedload(UserLog.user))\ .options(joinedload(UserLog.repository)) #filter - try: - journal = _journal_filter(journal, c.search_term) - except Exception: - # we want this to crash for now - raise + journal = _journal_filter(journal, c.search_term) journal = journal.filter(filtering_criterion)\ .order_by(UserLog.action_date.desc()) else: @@ -320,6 +317,7 @@ Session.commit() return 'ok' except Exception: + log.error(traceback.format_exc()) raise HTTPBadRequest() repo_id = request.POST.get('follows_repo_id') @@ -330,6 +328,7 @@ Session.commit() return 'ok' except Exception: + log.error(traceback.format_exc()) raise HTTPBadRequest() log.debug('token mismatch %s vs %s' % (cur_token, token))
--- a/kallithea/controllers/summary.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/controllers/summary.py Tue Jan 06 00:54:36 2015 +0100 @@ -118,8 +118,6 @@ pass except EmptyRepositoryError: pass - except Exception: - log.error(traceback.format_exc()) return readme_data, readme_file
--- a/kallithea/lib/auth.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/lib/auth.py Tue Jan 06 00:54:36 2015 +0100 @@ -711,8 +711,6 @@ sa = meta.Session all_perms = sa.query(Permission).all() config['available_permissions'] = [x.permission_name for x in all_perms] - except Exception: - log.error(traceback.format_exc()) finally: meta.Session.remove() @@ -1175,12 +1173,7 @@ # dict by unicode repo_name = safe_unicode(repo_name) usr = AuthUser(user.user_id) - try: - self.user_perms = set([usr.permissions['repositories'][repo_name]]) - except Exception: - log.error('Exception while accessing permissions %s' % - traceback.format_exc()) - self.user_perms = set() + self.user_perms = set([usr.permissions['repositories'][repo_name]]) self.username = user.username self.repo_name = repo_name return self.check_permissions() @@ -1318,15 +1311,8 @@ log.debug('checking if ip:%s is subnet of %s' % (source_ip, allowed_ips)) if isinstance(allowed_ips, (tuple, list, set)): for ip in allowed_ips: - try: - if ipaddr.IPAddress(source_ip) in ipaddr.IPNetwork(ip): - log.debug('IP %s is network %s' % - (ipaddr.IPAddress(source_ip), ipaddr.IPNetwork(ip))) - return True - # for any case we cannot determine the IP, don't crash just - # skip it and log as error, we want to say forbidden still when - # sending bad IP - except Exception: - log.error(traceback.format_exc()) - continue + if ipaddr.IPAddress(source_ip) in ipaddr.IPNetwork(ip): + log.debug('IP %s is network %s' % + (ipaddr.IPAddress(source_ip), ipaddr.IPNetwork(ip))) + return True return False
--- a/kallithea/lib/auth_modules/__init__.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/lib/auth_modules/__init__.py Tue Jan 06 00:54:36 2015 +0100 @@ -293,12 +293,8 @@ Session().flush() # enforce user is just in given groups, all of them has to be ones # created from plugins. We store this info in _group_data JSON field - try: - groups = auth['groups'] or [] - UserGroupModel().enforce_groups(user, groups, self.name) - except Exception: - # for any reason group syncing fails, we should proceed with login - log.error(traceback.format_exc()) + groups = auth['groups'] or [] + UserGroupModel().enforce_groups(user, groups, self.name) Session().commit() return auth
--- a/kallithea/lib/auth_modules/auth_pam.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/lib/auth_modules/auth_pam.py Tue Jan 06 00:54:36 2015 +0100 @@ -132,7 +132,7 @@ user_attrs["firstname"] = match.group('first_name') user_attrs["lastname"] = match.group('last_name') except Exception: - log.warning("Cannot extract additional info for PAM user") + log.warning("Cannot extract additional info for PAM user %s", username) pass log.debug("pamuser: \n%s" % formatted_json(user_attrs))
--- a/kallithea/lib/celerylib/tasks.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/lib/celerylib/tasks.py Tue Jan 06 00:54:36 2015 +0100 @@ -59,13 +59,10 @@ def get_logger(cls): if CELERY_ON: try: - log = cls.get_logger() - except Exception: - log = logging.getLogger(__name__) - else: - log = logging.getLogger(__name__) - - return log + return cls.get_logger() + except AttributeError: + pass + return logging.getLogger(__name__) @task(ignore_result=True)
--- a/kallithea/lib/db_manage.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/lib/db_manage.py Tue Jan 06 00:54:36 2015 +0100 @@ -190,12 +190,8 @@ paths.ui_value = paths.ui_value.replace('*', '') - try: - self.sa.add(paths) - self.sa.commit() - except Exception: - self.sa.rollback() - raise + self.sa.add(paths) + self.sa.commit() def fix_default_user(self): """ @@ -210,12 +206,8 @@ def_user.lastname = 'User' def_user.email = 'anonymous@kallithea-scm.org' - try: - self.sa.add(def_user) - self.sa.commit() - except Exception: - self.sa.rollback() - raise + self.sa.add(def_user) + self.sa.commit() def fix_settings(self): """ @@ -224,12 +216,8 @@ hgsettings3 = Setting('ga_code', '') - try: - self.sa.add(hgsettings3) - self.sa.commit() - except Exception: - self.sa.rollback() - raise + self.sa.add(hgsettings3) + self.sa.commit() def admin_prompt(self, second=False): if not self.tests:
--- a/kallithea/lib/helpers.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/lib/helpers.py Tue Jan 06 00:54:36 2015 +0100 @@ -1318,68 +1318,62 @@ return literal(newtext) def urlify_issues(newtext, repository, link_=None): - try: - import traceback - from kallithea import CONFIG - conf = CONFIG + from kallithea import CONFIG as conf - # allow multiple issue servers to be used - valid_indices = [ - x.group(1) - for x in map(lambda x: re.match(r'issue_pat(.*)', x), conf.keys()) - if x and 'issue_server_link%s' % x.group(1) in conf - and 'issue_prefix%s' % x.group(1) in conf - ] + # allow multiple issue servers to be used + valid_indices = [ + x.group(1) + for x in map(lambda x: re.match(r'issue_pat(.*)', x), conf.keys()) + if x and 'issue_server_link%s' % x.group(1) in conf + and 'issue_prefix%s' % x.group(1) in conf + ] - if valid_indices: - log.debug('found issue server suffixes `%s` during valuation of: %s' - % (','.join(valid_indices), newtext)) + if valid_indices: + log.debug('found issue server suffixes `%s` during valuation of: %s' + % (','.join(valid_indices), newtext)) - for pattern_index in valid_indices: - ISSUE_PATTERN = conf.get('issue_pat%s' % pattern_index) - ISSUE_SERVER_LNK = conf.get('issue_server_link%s' % pattern_index) - ISSUE_PREFIX = conf.get('issue_prefix%s' % pattern_index) + for pattern_index in valid_indices: + ISSUE_PATTERN = conf.get('issue_pat%s' % pattern_index) + ISSUE_SERVER_LNK = conf.get('issue_server_link%s' % pattern_index) + ISSUE_PREFIX = conf.get('issue_prefix%s' % pattern_index) - log.debug('pattern suffix `%s` PAT:%s SERVER_LINK:%s PREFIX:%s' - % (pattern_index, ISSUE_PATTERN, ISSUE_SERVER_LNK, - ISSUE_PREFIX)) + log.debug('pattern suffix `%s` PAT:%s SERVER_LINK:%s PREFIX:%s' + % (pattern_index, ISSUE_PATTERN, ISSUE_SERVER_LNK, + ISSUE_PREFIX)) - URL_PAT = re.compile(r'%s' % ISSUE_PATTERN) + URL_PAT = re.compile(r'%s' % ISSUE_PATTERN) - def url_func(match_obj): - pref = '' - if match_obj.group().startswith(' '): - pref = ' ' + def url_func(match_obj): + pref = '' + if match_obj.group().startswith(' '): + pref = ' ' - issue_id = ''.join(match_obj.groups()) - issue_url = ISSUE_SERVER_LNK.replace('{id}', issue_id) - if repository: - issue_url = issue_url.replace('{repo}', repository) - repo_name = repository.split(URL_SEP)[-1] - issue_url = issue_url.replace('{repo_name}', repo_name) + issue_id = ''.join(match_obj.groups()) + issue_url = ISSUE_SERVER_LNK.replace('{id}', issue_id) + if repository: + issue_url = issue_url.replace('{repo}', repository) + repo_name = repository.split(URL_SEP)[-1] + issue_url = issue_url.replace('{repo_name}', repo_name) - return ( - '%(pref)s<a class="%(cls)s" href="%(url)s">' - '%(issue-prefix)s%(id-repr)s' - '</a>' - ) % { - 'pref': pref, - 'cls': 'issue-tracker-link', - 'url': issue_url, - 'id-repr': issue_id, - 'issue-prefix': ISSUE_PREFIX, - 'serv': ISSUE_SERVER_LNK, - } - newtext = URL_PAT.sub(url_func, newtext) - log.debug('processed prefix:`%s` => %s' % (pattern_index, newtext)) + return ( + '%(pref)s<a class="%(cls)s" href="%(url)s">' + '%(issue-prefix)s%(id-repr)s' + '</a>' + ) % { + 'pref': pref, + 'cls': 'issue-tracker-link', + 'url': issue_url, + 'id-repr': issue_id, + 'issue-prefix': ISSUE_PREFIX, + 'serv': ISSUE_SERVER_LNK, + } + newtext = URL_PAT.sub(url_func, newtext) + log.debug('processed prefix:`%s` => %s' % (pattern_index, newtext)) - # if we actually did something above - if link_: - # wrap not links into final link => link_ - newtext = linkify_others(newtext, link_) - except Exception: - log.error(traceback.format_exc()) - pass + # if we actually did something above + if link_: + # wrap not links into final link => link_ + newtext = linkify_others(newtext, link_) return newtext
--- a/kallithea/lib/utils.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/lib/utils.py Tue Jan 06 00:54:36 2015 +0100 @@ -120,16 +120,10 @@ def get_user_group_slug(request): _group = request.environ['pylons.routes_dict'].get('id') - try: - _group = UserGroup.get(_group) - if _group: - _group = _group.users_group_name - except Exception: - log.debug(traceback.format_exc()) - #catch all failures here - pass - - return _group + _group = UserGroup.get(_group) + if _group: + return _group.users_group_name + return None def _extract_id_from_repo_name(repo_name): @@ -147,15 +141,14 @@ :param repo_name: :return: repo_name if matched else None """ - try: - _repo_id = _extract_id_from_repo_name(repo_name) - if _repo_id: - from kallithea.model.db import Repository - return Repository.get(_repo_id).repo_name - except Exception: - log.debug('Failed to extract repo_name from URL %s' % ( - traceback.format_exc())) - return + _repo_id = _extract_id_from_repo_name(repo_name) + if _repo_id: + from kallithea.model.db import Repository + repo = Repository.get(_repo_id) + if repo: + # TODO: return repo instead of reponame? or would that be a layering violation? + return repo.repo_name + return None def action_logger(user, action, repo, ipaddr='', sa=None, commit=False): @@ -180,43 +173,39 @@ if not ipaddr: ipaddr = getattr(get_current_authuser(), 'ip_addr', '') - try: - if getattr(user, 'user_id', None): - user_obj = User.get(user.user_id) - elif isinstance(user, basestring): - user_obj = User.get_by_username(user) - else: - raise Exception('You have to provide a user object or a username') + if getattr(user, 'user_id', None): + user_obj = User.get(user.user_id) + elif isinstance(user, basestring): + user_obj = User.get_by_username(user) + else: + raise Exception('You have to provide a user object or a username') - if getattr(repo, 'repo_id', None): - repo_obj = Repository.get(repo.repo_id) - repo_name = repo_obj.repo_name - elif isinstance(repo, basestring): - repo_name = repo.lstrip('/') - repo_obj = Repository.get_by_repo_name(repo_name) - else: - repo_obj = None - repo_name = '' + if getattr(repo, 'repo_id', None): + repo_obj = Repository.get(repo.repo_id) + repo_name = repo_obj.repo_name + elif isinstance(repo, basestring): + repo_name = repo.lstrip('/') + repo_obj = Repository.get_by_repo_name(repo_name) + else: + repo_obj = None + repo_name = '' - user_log = UserLog() - user_log.user_id = user_obj.user_id - user_log.username = user_obj.username - user_log.action = safe_unicode(action) + user_log = UserLog() + user_log.user_id = user_obj.user_id + user_log.username = user_obj.username + user_log.action = safe_unicode(action) - user_log.repository = repo_obj - user_log.repository_name = repo_name + user_log.repository = repo_obj + user_log.repository_name = repo_name - user_log.action_date = datetime.datetime.now() - user_log.user_ip = ipaddr - sa.add(user_log) + user_log.action_date = datetime.datetime.now() + user_log.user_ip = ipaddr + sa.add(user_log) - log.info('Logging action:%s on %s by user:%s ip:%s' % - (action, safe_unicode(repo), user_obj, ipaddr)) - if commit: - sa.commit() - except Exception: - log.error(traceback.format_exc()) - raise + log.info('Logging action:%s on %s by user:%s ip:%s' % + (action, safe_unicode(repo), user_obj, ipaddr)) + if commit: + sa.commit() def get_filesystem_repos(path, recursive=False, skip_removed_repos=True): @@ -816,7 +805,7 @@ ver = '.'.join(ver.split('.')[:3]) try: _ver = StrictVersion(ver) - except Exception: + except ValueError: _ver = StrictVersion('0.0.0') stderr = traceback.format_exc()
--- a/kallithea/lib/utils2.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/lib/utils2.py Tue Jan 06 00:54:36 2015 +0100 @@ -32,7 +32,6 @@ import time import uuid import datetime -import traceback import webob import urllib import urlobject @@ -596,14 +595,14 @@ def obfuscate_url_pw(engine): - _url = engine or '' from sqlalchemy.engine import url as sa_url + from sqlalchemy.exc import ArgumentError try: - _url = sa_url.make_url(engine) - if _url.password: - _url.password = 'XXXXX' - except Exception: - pass + _url = sa_url.make_url(engine or '') + except ArgumentError: + return engine + if _url.password: + _url.password = 'XXXXX' return str(_url) @@ -622,9 +621,7 @@ try: rc_extras = json.loads(env['KALLITHEA_EXTRAS']) - except Exception: - print os.environ - print >> sys.stderr, traceback.format_exc() + except KeyError: rc_extras = {} try:
--- a/kallithea/lib/vcs/backends/git/repository.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/lib/vcs/backends/git/repository.py Tue Jan 06 00:54:36 2015 +0100 @@ -294,7 +294,7 @@ or isinstance(revision, int) or is_null(revision)): try: revision = self.revisions[int(revision)] - except Exception: + except IndexError: msg = ("Revision %s does not exist for %s" % (revision, self)) raise ChangesetDoesNotExistError(msg)
--- a/kallithea/lib/vcs/backends/hg/repository.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/lib/vcs/backends/hg/repository.py Tue Jan 06 00:54:36 2015 +0100 @@ -344,13 +344,8 @@ opts = {} if not update_after_clone: opts.update({'noupdate': True}) - try: - MercurialRepository._check_url(url, self.baseui) - clone(self.baseui, url, self.path, **opts) -# except urllib2.URLError: -# raise Abort("Got HTTP 404 error") - except Exception: - raise + MercurialRepository._check_url(url, self.baseui) + clone(self.baseui, url, self.path, **opts) # Don't try to create if we've already cloned repo create = False
--- a/kallithea/lib/vcs/nodes.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/lib/vcs/nodes.py Tue Jan 06 00:54:36 2015 +0100 @@ -340,9 +340,9 @@ #try with pygments try: - from pygments.lexers import get_lexer_for_filename - mt = get_lexer_for_filename(self.name).mimetypes - except Exception: + from pygments import lexers + mt = lexers.get_lexer_for_filename(self.name).mimetypes + except lexers.ClassNotFound: mt = None if mt:
--- a/kallithea/lib/vcs/subprocessio.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/lib/vcs/subprocessio.py Tue Jan 06 00:54:36 2015 +0100 @@ -45,15 +45,9 @@ else: # can be either file pointer or file-like if type(source) in (int, long): # file pointer it is ## converting file descriptor (int) stdin into file-like - try: - source = os.fdopen(source, 'rb', 16384) - except Exception: - pass + source = os.fdopen(source, 'rb', 16384) # let's see if source is file-like by now - try: - filelike = source.read - except Exception: - pass + filelike = hasattr(source, 'read') if not filelike and not self.bytes: raise TypeError("StreamFeeder's source object must be a readable " "file-like, a file descriptor, or a string-like.")
--- a/kallithea/model/api_key.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/model/api_key.py Tue Jan 06 00:54:36 2015 +0100 @@ -28,7 +28,6 @@ from __future__ import with_statement import time import logging -import traceback from sqlalchemy import or_ from kallithea.lib.utils2 import generate_api_key @@ -71,11 +70,7 @@ api_key = api_key.filter(UserApiKeys.user_id == user.user_id) api_key = api_key.scalar() - try: - Session().delete(api_key) - except Exception: - log.error(traceback.format_exc()) - raise + Session().delete(api_key) def get_api_keys(self, user, show_expired=True): user = self._get_user(user)
--- a/kallithea/model/db.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/model/db.py Tue Jan 06 00:54:36 2015 +0100 @@ -1266,7 +1266,7 @@ try: from pylons import tmpl_context as c uri_tmpl = c.clone_uri_tmpl - except Exception: + except AttributeError: # in any case if we call this outside of request context, # ie, not having tmpl_context set up pass @@ -2091,19 +2091,16 @@ inv_objs = Session().query(cls).filter(cls.cache_args == repo_name).all() log.debug('for repo %s got %s invalidation objects' % (safe_str(repo_name), inv_objs)) - try: - for inv_obj in inv_objs: - log.debug('marking %s key for invalidation based on repo_name=%s' - % (inv_obj, safe_str(repo_name))) - if delete: - Session().delete(inv_obj) - else: - inv_obj.cache_active = False - Session().add(inv_obj) - Session().commit() - except Exception: - log.error(traceback.format_exc()) - Session().rollback() + + for inv_obj in inv_objs: + log.debug('marking %s key for invalidation based on repo_name=%s' + % (inv_obj, safe_str(repo_name))) + if delete: + Session().delete(inv_obj) + else: + inv_obj.cache_active = False + Session().add(inv_obj) + Session().commit() @classmethod def test_and_set_valid(cls, repo_name, kind, valid_cache_keys=None): @@ -2119,20 +2116,15 @@ if valid_cache_keys and cache_key in valid_cache_keys: return True - try: - inv_obj = cls.query().filter(cls.cache_key == cache_key).scalar() - if not inv_obj: - inv_obj = CacheInvalidation(cache_key, repo_name) - if inv_obj.cache_active: - return True - inv_obj.cache_active = True - Session().add(inv_obj) - Session().commit() - return False - except Exception: - log.error(traceback.format_exc()) - Session().rollback() - return False + inv_obj = cls.query().filter(cls.cache_key == cache_key).scalar() + if not inv_obj: + inv_obj = CacheInvalidation(cache_key, repo_name) + if inv_obj.cache_active: + return True + inv_obj.cache_active = True + Session().add(inv_obj) + Session().commit() + return False @classmethod def get_valid_cache_keys(cls):
--- a/kallithea/model/user.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/model/user.py Tue Jan 06 00:54:36 2015 +0100 @@ -91,23 +91,20 @@ # raises UserCreationError if it's not allowed check_allowed_create_user(user_data, cur_user) from kallithea.lib.auth import get_crypt_password - try: - new_user = User() - for k, v in form_data.items(): - if k == 'password': - v = get_crypt_password(v) - if k == 'firstname': - k = 'name' - setattr(new_user, k, v) - new_user.api_key = generate_api_key(form_data['username']) - self.sa.add(new_user) + new_user = User() + for k, v in form_data.items(): + if k == 'password': + v = get_crypt_password(v) + if k == 'firstname': + k = 'name' + setattr(new_user, k, v) - log_create_user(new_user.get_dict(), cur_user) - return new_user - except Exception: - log.error(traceback.format_exc()) - raise + new_user.api_key = generate_api_key(form_data['username']) + self.sa.add(new_user) + + log_create_user(new_user.get_dict(), cur_user) + return new_user def create_or_update(self, username, password, email, firstname='', lastname='', active=True, admin=False, @@ -185,104 +182,90 @@ from kallithea.model.notification import NotificationModel import kallithea.lib.helpers as h - try: - form_data['admin'] = False - form_data['extern_name'] = EXTERN_TYPE_INTERNAL - form_data['extern_type'] = EXTERN_TYPE_INTERNAL - new_user = self.create(form_data) + form_data['admin'] = False + form_data['extern_name'] = EXTERN_TYPE_INTERNAL + form_data['extern_type'] = EXTERN_TYPE_INTERNAL + new_user = self.create(form_data) - self.sa.add(new_user) - self.sa.flush() + self.sa.add(new_user) + self.sa.flush() - # notification to admins - subject = _('New user registration') - body = ('New user registration\n' - '---------------------\n' - '- Username: %s\n' - '- Full Name: %s\n' - '- Email: %s\n') - body = body % (new_user.username, new_user.full_name, new_user.email) - edit_url = h.canonical_url('edit_user', id=new_user.user_id) - email_kwargs = {'registered_user_url': edit_url, 'new_username': new_user.username} - NotificationModel().create(created_by=new_user, subject=subject, - body=body, recipients=None, - type_=Notification.TYPE_REGISTRATION, - email_kwargs=email_kwargs) - except Exception: - log.error(traceback.format_exc()) - raise + # notification to admins + subject = _('New user registration') + body = ('New user registration\n' + '---------------------\n' + '- Username: %s\n' + '- Full Name: %s\n' + '- Email: %s\n') + body = body % (new_user.username, new_user.full_name, new_user.email) + edit_url = h.canonical_url('edit_user', id=new_user.user_id) + email_kwargs = {'registered_user_url': edit_url, 'new_username': new_user.username} + NotificationModel().create(created_by=new_user, subject=subject, + body=body, recipients=None, + type_=Notification.TYPE_REGISTRATION, + email_kwargs=email_kwargs) def update(self, user_id, form_data, skip_attrs=[]): from kallithea.lib.auth import get_crypt_password - try: - user = self.get(user_id, cache=False) - if user.username == User.DEFAULT_USER: - raise DefaultUserException( - _("You can't Edit this user since it's " - "crucial for entire application")) + + user = self.get(user_id, cache=False) + if user.username == User.DEFAULT_USER: + raise DefaultUserException( + _("You can't Edit this user since it's " + "crucial for entire application")) - for k, v in form_data.items(): - if k in skip_attrs: - continue - if k == 'new_password' and v: - user.password = get_crypt_password(v) - else: - # old legacy thing orm models store firstname as name, - # need proper refactor to username - if k == 'firstname': - k = 'name' - setattr(user, k, v) - self.sa.add(user) - except Exception: - log.error(traceback.format_exc()) - raise + for k, v in form_data.items(): + if k in skip_attrs: + continue + if k == 'new_password' and v: + user.password = get_crypt_password(v) + else: + # old legacy thing orm models store firstname as name, + # need proper refactor to username + if k == 'firstname': + k = 'name' + setattr(user, k, v) + self.sa.add(user) def update_user(self, user, **kwargs): from kallithea.lib.auth import get_crypt_password - try: - user = self._get_user(user) - if user.username == User.DEFAULT_USER: - raise DefaultUserException( - _("You can't Edit this user since it's" - " crucial for entire application") - ) + + user = self._get_user(user) + if user.username == User.DEFAULT_USER: + raise DefaultUserException( + _("You can't Edit this user since it's" + " crucial for entire application") + ) - for k, v in kwargs.items(): - if k == 'password' and v: - v = get_crypt_password(v) + for k, v in kwargs.items(): + if k == 'password' and v: + v = get_crypt_password(v) - setattr(user, k, v) - self.sa.add(user) - return user - except Exception: - log.error(traceback.format_exc()) - raise + setattr(user, k, v) + self.sa.add(user) + return user def delete(self, user, cur_user=None): if not cur_user: cur_user = getattr(get_current_authuser(), 'username', None) user = self._get_user(user) - try: - if user.username == User.DEFAULT_USER: - raise DefaultUserException( - _(u"You can't remove this user since it's" - " crucial for entire application") - ) - if user.repositories: - repos = [x.repo_name for x in user.repositories] - raise UserOwnsReposException( - _(u'user "%s" still owns %s repositories and cannot be ' - 'removed. Switch owners or remove those repositories. %s') - % (user.username, len(repos), ', '.join(repos)) - ) - self.sa.delete(user) + if user.username == User.DEFAULT_USER: + raise DefaultUserException( + _(u"You can't remove this user since it's" + " crucial for entire application") + ) + if user.repositories: + repos = [x.repo_name for x in user.repositories] + raise UserOwnsReposException( + _(u'user "%s" still owns %s repositories and cannot be ' + 'removed. Switch owners or remove those repositories. %s') + % (user.username, len(repos), ', '.join(repos)) + ) + self.sa.delete(user) - from kallithea.lib.hooks import log_delete_user - log_delete_user(user.get_dict(), cur_user) - except Exception: - log.error(traceback.format_exc()) - raise + from kallithea.lib.hooks import log_delete_user + log_delete_user(user.get_dict(), cur_user) def reset_password_link(self, data): from kallithea.lib.celerylib import tasks, run_task @@ -290,29 +273,25 @@ import kallithea.lib.helpers as h user_email = data['email'] - try: - user = User.get_by_email(user_email) - if user: - log.debug('password reset user found %s' % user) - link = h.canonical_url('reset_password_confirmation', key=user.api_key) - reg_type = EmailNotificationModel.TYPE_PASSWORD_RESET - body = EmailNotificationModel().get_email_tmpl(reg_type, - 'txt', - user=user.short_contact, - reset_url=link) - html_body = EmailNotificationModel().get_email_tmpl(reg_type, - 'html', - user=user.short_contact, - reset_url=link) - log.debug('sending email') - run_task(tasks.send_email, [user_email], - _("Password reset link"), body, html_body) - log.info('send new password mail to %s' % user_email) - else: - log.debug("password reset email %s not found" % user_email) - except Exception: - log.error(traceback.format_exc()) - return False + user = User.get_by_email(user_email) + if user: + log.debug('password reset user found %s' % user) + link = h.canonical_url('reset_password_confirmation', key=user.api_key) + reg_type = EmailNotificationModel.TYPE_PASSWORD_RESET + body = EmailNotificationModel().get_email_tmpl(reg_type, + 'txt', + user=user.short_contact, + reset_url=link) + html_body = EmailNotificationModel().get_email_tmpl(reg_type, + 'html', + user=user.short_contact, + reset_url=link) + log.debug('sending email') + run_task(tasks.send_email, [user_email], + _("Password reset link"), body, html_body) + log.info('send new password mail to %s' % user_email) + else: + log.debug("password reset email %s not found" % user_email) return True @@ -320,32 +299,21 @@ from kallithea.lib.celerylib import tasks, run_task from kallithea.lib import auth user_email = data['email'] - pre_db = True - try: - user = User.get_by_email(user_email) - new_passwd = auth.PasswordGenerator().gen_password(8, - auth.PasswordGenerator.ALPHABETS_BIG_SMALL) - if user: - user.password = auth.get_crypt_password(new_passwd) - Session().add(user) - Session().commit() - log.info('change password for %s' % user_email) - if new_passwd is None: - raise Exception('unable to generate new password') + user = User.get_by_email(user_email) + new_passwd = auth.PasswordGenerator().gen_password(8, + auth.PasswordGenerator.ALPHABETS_BIG_SMALL) + if user: + user.password = auth.get_crypt_password(new_passwd) + Session().add(user) + Session().commit() + log.info('change password for %s' % user_email) + if new_passwd is None: + raise Exception('unable to generate new password') - pre_db = False - run_task(tasks.send_email, [user_email], - _('Your new password'), - _('Your new Kallithea password:%s') % (new_passwd,)) - log.info('send new password mail to %s' % user_email) - - except Exception: - log.error('Failed to update user password') - log.error(traceback.format_exc()) - if pre_db: - # we rollback only if local db stuff fails. If it goes into - # run_task, we're pass rollback state this wouldn't work then - Session().rollback() + run_task(tasks.send_email, [user_email], + _('Your new password'), + _('Your new Kallithea password:%s') % (new_passwd,)) + log.info('send new password mail to %s' % user_email) return True @@ -364,29 +332,21 @@ if user_id is None and api_key is None and username is None: raise Exception('You need to pass user_id, api_key or username') - try: - dbuser = None - if user_id: - dbuser = self.get(user_id) - elif api_key: - dbuser = self.get_by_api_key(api_key) - elif username: - dbuser = self.get_by_username(username) + dbuser = None + if user_id: + dbuser = self.get(user_id) + elif api_key: + dbuser = self.get_by_api_key(api_key) + elif username: + dbuser = self.get_by_username(username) - if dbuser is not None and dbuser.active: - log.debug('filling %s data' % dbuser) - for k, v in dbuser.get_dict().iteritems(): - if k not in ['api_keys', 'permissions']: - setattr(auth_user, k, v) - else: - return False - - except Exception: - log.error(traceback.format_exc()) - auth_user.is_authenticated = False - return False - - return True + if dbuser is not None and dbuser.active: + log.debug('filling %s data' % dbuser) + for k, v in dbuser.get_dict().iteritems(): + if k not in ['api_keys', 'permissions']: + setattr(auth_user, k, v) + return True + return False def has_perm(self, user, perm): perm = self._get_perm(perm)
--- a/kallithea/model/validators.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/model/validators.py Tue Jan 06 00:54:36 2015 +0100 @@ -22,6 +22,7 @@ from collections import defaultdict from pylons.i18n.translation import _ from webhelpers.pylonslib.secure_form import authentication_token +import sqlalchemy from formencode.validators import ( UnicodeString, OneOf, Int, Number, Regex, Email, Bool, StringBoolean, Set, @@ -142,7 +143,7 @@ try: User.query().filter(User.active == True)\ .filter(User.username == value).one() - except Exception: + except sqlalchemy.exc.InvalidRequestError: # NoResultFound/MultipleResultsFound msg = M(self, 'invalid_username', state, username=value) raise formencode.Invalid(msg, value, state, error_dict=dict(username=msg)
--- a/kallithea/tests/functional/test_admin_repos.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/tests/functional/test_admin_repos.py Tue Jan 06 00:54:36 2015 +0100 @@ -79,7 +79,7 @@ # test if the repository was created on filesystem try: vcs.get_repo(os.path.join(TESTS_TMP_PATH, repo_name)) - except Exception: + except vcs.exceptions.VCSError: self.fail('no repo %s in filesystem' % repo_name) RepoModel().delete(repo_name) @@ -118,7 +118,7 @@ # test if the repository was created on filesystem try: vcs.get_repo(os.path.join(TESTS_TMP_PATH, repo_name)) - except Exception: + except vcs.exceptions.VCSError: self.fail('no repo %s in filesystem' % repo_name) def test_create_in_group(self): @@ -166,7 +166,7 @@ # test if the repository was created on filesystem try: vcs.get_repo(os.path.join(TESTS_TMP_PATH, repo_name_full)) - except Exception: + except vcs.exceptions.VCSError: RepoGroupModel().delete(group_name) Session().commit() self.fail('no repo %s in filesystem' % repo_name) @@ -254,7 +254,7 @@ # test if the repository was created on filesystem try: vcs.get_repo(os.path.join(TESTS_TMP_PATH, repo_name_full)) - except Exception: + except vcs.exceptions.VCSError: RepoGroupModel().delete(group_name) Session().commit() self.fail('no repo %s in filesystem' % repo_name) @@ -310,7 +310,7 @@ # test if the repository was created on filesystem try: vcs.get_repo(os.path.join(TESTS_TMP_PATH, repo_name_full)) - except Exception: + except vcs.exceptions.VCSError: RepoGroupModel().delete(group_name) Session().commit() self.fail('no repo %s in filesystem' % repo_name) @@ -384,7 +384,7 @@ # test if the repository was created on filesystem try: vcs.get_repo(os.path.join(TESTS_TMP_PATH, repo_name)) - except Exception: + except vcs.exceptions.VCSError: self.fail('no repo %s in filesystem' % repo_name) response = self.app.delete(url('repo', repo_name=repo_name)) @@ -435,7 +435,7 @@ # test if the repository was created on filesystem try: vcs.get_repo(os.path.join(TESTS_TMP_PATH, repo_name)) - except Exception: + except vcs.exceptions.VCSError: self.fail('no repo %s in filesystem' % repo_name) response = self.app.delete(url('repo', repo_name=repo_name))
--- a/kallithea/tests/scripts/test_concurency.py Tue Jan 06 00:54:36 2015 +0100 +++ b/kallithea/tests/scripts/test_concurency.py Tue Jan 06 00:54:36 2015 +0100 @@ -200,12 +200,12 @@ try: METHOD = sys.argv[3] - except Exception: + except IndexError: pass try: backend = sys.argv[4] - except Exception: + except IndexError: backend = 'hg' if METHOD == 'pull':