From b1fc61ec5e9bdbf237a402656910856c079d36cb Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Jun 05 2015 08:56:15 +0000
Subject: Project wide pylint clean up and bug fixes


---

diff --git a/pagure/__init__.py b/pagure/__init__.py
index bc256e5..c89e6e0 100644
--- a/pagure/__init__.py
+++ b/pagure/__init__.py
@@ -157,7 +157,7 @@ def generate_gitolite_acls():
             raise pagure.exceptions.PagureException(
                 'Non-supported gitolite version "%s"' % gitolite_version
             )
-        proc = subprocess.Popen(
+        subprocess.Popen(
             cmd,
             shell=True,
             stdout=subprocess.PIPE,
diff --git a/pagure/api/__init__.py b/pagure/api/__init__.py
index 146920c..49b4d83 100644
--- a/pagure/api/__init__.py
+++ b/pagure/api/__init__.py
@@ -59,7 +59,6 @@ def check_api_acls(acls, optional=False):
     flask.g.user = None
     token = None
     token_str = None
-    apt_login = None
     if 'Authorization' in flask.request.headers:
         authorization = flask.request.headers['Authorization']
         if 'token' in authorization:
@@ -270,8 +269,8 @@ def api_project_tags(repo, username=None):
     if pattern is not None and not pattern.endswith('*'):
         pattern += '*'
 
-    project = pagure.lib.get_project(SESSION, repo, username)
-    if not project:
+    project_obj = pagure.lib.get_project(SESSION, repo, username)
+    if not project_obj:
         output = {'output': 'notok', 'error': 'Project not found'}
         jsonout = flask.jsonify(output)
         jsonout.status_code = 404
@@ -282,7 +281,7 @@ def api_project_tags(repo, username=None):
             'tags': [
                 tag.tag
                 for tag in pagure.lib.get_tags_of_project(
-                    SESSION, project, pattern=pattern)
+                    SESSION, project_obj, pattern=pattern)
             ]
         }
     )
diff --git a/pagure/api/fork.py b/pagure/api/fork.py
index 61aae66..5c81001 100644
--- a/pagure/api/fork.py
+++ b/pagure/api/fork.py
@@ -15,10 +15,8 @@ from sqlalchemy.exc import SQLAlchemyError
 import pagure
 import pagure.exceptions
 import pagure.lib
-from pagure import APP, SESSION, is_repo_admin, authenticated
-from pagure.api import (
-    API, api_method, api_login_required, api_login_optional, APIERROR
-)
+from pagure import APP, SESSION, is_repo_admin
+from pagure.api import API, api_method, api_login_required, APIERROR
 
 
 @API.route('/<repo>/pull-requests')
@@ -102,7 +100,6 @@ def api_pull_request_views(repo, username=None):
     """
 
     repo = pagure.lib.get_project(SESSION, repo, user=username)
-    output = {}
 
     if repo is None:
         raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT)
@@ -209,7 +206,6 @@ def api_pull_request_view(repo, requestid, username=None):
     """
 
     repo = pagure.lib.get_project(SESSION, repo, user=username)
-    output = {}
 
     if repo is None:
         raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT)
diff --git a/pagure/api/issue.py b/pagure/api/issue.py
index ec50716..0253e1f 100644
--- a/pagure/api/issue.py
+++ b/pagure/api/issue.py
@@ -114,6 +114,7 @@ def api_new_issue(repo, username=None):
             output['message'] = 'Issue created'
         except SQLAlchemyError, err:  # pragma: no cover
             SESSION.rollback()
+            APP.logger.exception(err)
             raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR)
 
     else:
@@ -250,7 +251,6 @@ def api_view_issues(repo, username=None):
     """
 
     repo = pagure.lib.get_project(SESSION, repo, user=username)
-    output = {}
 
     if repo is None:
         raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT)
@@ -359,7 +359,6 @@ def api_view_issue(repo, issueid, username=None):
     """
 
     repo = pagure.lib.get_project(SESSION, repo, user=username)
-    output = {}
 
     if repo is None:
         raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT)
@@ -542,6 +541,7 @@ def api_comment_issue(repo, issueid, username=None):
             output['message'] = message
         except SQLAlchemyError, err:  # pragma: no cover
             SESSION.rollback()
+            APP.logger.exception(err)
             raise pagure.exceptions.APIError(400, error_code=APIERROR.EDBERROR)
 
     else:
diff --git a/pagure/api/project.py b/pagure/api/project.py
index 5a3ea11..9d03a12 100644
--- a/pagure/api/project.py
+++ b/pagure/api/project.py
@@ -13,10 +13,8 @@ import flask
 import pagure
 import pagure.exceptions
 import pagure.lib
-from pagure import APP, SESSION, is_repo_admin, authenticated
-from pagure.api import (
-    API, api_method, api_login_required, api_login_optional, APIERROR
-)
+from pagure import SESSION
+from pagure.api import API, api_method, APIERROR
 
 
 @API.route('/<repo>/git/tags')
@@ -45,7 +43,6 @@ def api_git_tags(repo, username=None):
 
     """
     repo = pagure.lib.get_project(SESSION, repo, user=username)
-    output = {}
 
     if repo is None:
         raise pagure.exceptions.APIError(404, error_code=APIERROR.ENOPROJECT)
diff --git a/pagure/api/user.py b/pagure/api/user.py
index d7d1eba..d6841be 100644
--- a/pagure/api/user.py
+++ b/pagure/api/user.py
@@ -13,10 +13,8 @@ import flask
 import pagure
 import pagure.exceptions
 import pagure.lib
-from pagure import APP, SESSION, is_repo_admin, authenticated
-from pagure.api import (
-    API, api_method, api_login_required, api_login_optional, APIERROR
-)
+from pagure import APP, SESSION
+from pagure.api import API, api_method, APIERROR
 
 
 @API.route('/user/<username>')
diff --git a/pagure/hooks/__init__.py b/pagure/hooks/__init__.py
index 411565d..f004f25 100644
--- a/pagure/hooks/__init__.py
+++ b/pagure/hooks/__init__.py
@@ -16,24 +16,24 @@ from pagure import APP, get_repo_path
 
 
 class RequiredIf(wtforms.validators.Required):
-        """ Wtforms validator setting a field as required if another field
-        has a value.
-        """
-
-        def __init__(self, fields, *args, **kwargs):
-            if isinstance(fields, basestring):
-                fields = [fields]
-            self.fields = fields
-            super(RequiredIf, self).__init__(*args, **kwargs)
-
-        def __call__(self, form, field):
-            for fieldname in self.fields:
-                nfield = form._fields.get(fieldname)
-                if nfield is None:
-                    raise Exception(
-                        'no field named "%s" in form' % fieldname)
-                if bool(nfield.data):
-                    super(RequiredIf, self).__call__(form, field)
+    """ Wtforms validator setting a field as required if another field
+    has a value.
+    """
+
+    def __init__(self, fields, *args, **kwargs):
+        if isinstance(fields, basestring):
+            fields = [fields]
+        self.fields = fields
+        super(RequiredIf, self).__init__(*args, **kwargs)
+
+    def __call__(self, form, field):
+        for fieldname in self.fields:
+            nfield = form._fields.get(fieldname)
+            if nfield is None:
+                raise Exception(
+                    'no field named "%s" in form' % fieldname)
+            if bool(nfield.data):
+                super(RequiredIf, self).__call__(form, field)
 
 
 class BaseHook(object):
diff --git a/pagure/hooks/fedmsg.py b/pagure/hooks/fedmsg.py
index be6f133..75fa30e 100644
--- a/pagure/hooks/fedmsg.py
+++ b/pagure/hooks/fedmsg.py
@@ -9,18 +9,16 @@
 """
 
 import os
-import shutil
 
 import sqlalchemy as sa
-import pygit2
 import wtforms
 from flask.ext import wtf
 from sqlalchemy.orm import relation
 from sqlalchemy.orm import backref
 
-from pagure.hooks import BaseHook, RequiredIf
+from pagure.hooks import BaseHook
 from pagure.lib.model import BASE, Project
-from pagure import SESSION, APP, get_repo_path
+from pagure import get_repo_path
 
 
 class FedmsgTable(BASE):
diff --git a/pagure/hooks/irc.py b/pagure/hooks/irc.py
index e976a01..a1f88ff 100644
--- a/pagure/hooks/irc.py
+++ b/pagure/hooks/irc.py
@@ -19,7 +19,7 @@ from sqlalchemy.orm import backref
 
 from pagure.hooks import BaseHook, RequiredIf
 from pagure.lib.model import BASE, Project
-from pagure import SESSION, APP, get_repo_path
+from pagure import get_repo_path
 
 
 class IrcTable(BASE):
diff --git a/pagure/hooks/mail.py b/pagure/hooks/mail.py
index 90cdfca..ff0053c 100644
--- a/pagure/hooks/mail.py
+++ b/pagure/hooks/mail.py
@@ -9,7 +9,6 @@
 """
 
 import os
-import shutil
 
 import sqlalchemy as sa
 import pygit2
@@ -20,7 +19,7 @@ from sqlalchemy.orm import backref
 
 from pagure.hooks import BaseHook, RequiredIf
 from pagure.lib.model import BASE, Project
-from pagure import SESSION, APP, get_repo_path
+from pagure import get_repo_path
 
 
 class MailTable(BASE):
diff --git a/pagure/hooks/pagure_hook.py b/pagure/hooks/pagure_hook.py
index 04eb686..2f8898c 100644
--- a/pagure/hooks/pagure_hook.py
+++ b/pagure/hooks/pagure_hook.py
@@ -9,7 +9,6 @@
 """
 
 import os
-import shutil
 
 import sqlalchemy as sa
 import pygit2
@@ -20,7 +19,7 @@ from sqlalchemy.orm import backref
 
 from pagure.hooks import BaseHook
 from pagure.lib.model import BASE, Project
-from pagure import SESSION, APP, get_repo_path
+from pagure import APP, get_repo_path
 
 
 class PagureTable(BASE):
@@ -88,7 +87,7 @@ class PagureHook(BaseHook):
 
         for repopath in repopaths:
             # Init the git repo in case
-            repo_obj = pygit2.Repository(repopath)
+            pygit2.Repository(repopath)
 
             # Install the hook itself
             hook_path = os.path.join(
diff --git a/pagure/hooks/pagure_request_hook.py b/pagure/hooks/pagure_request_hook.py
index 83421b7..b900e39 100644
--- a/pagure/hooks/pagure_request_hook.py
+++ b/pagure/hooks/pagure_request_hook.py
@@ -9,7 +9,6 @@
 """
 
 import os
-import shutil
 
 import flask
 import sqlalchemy as sa
@@ -21,7 +20,7 @@ from sqlalchemy.orm import backref
 
 from pagure.hooks import BaseHook
 from pagure.lib.model import BASE, Project
-from pagure import SESSION, APP
+from pagure import APP
 
 
 class PagureRequestsTable(BASE):
@@ -107,7 +106,7 @@ class PagureRequestHook(BaseHook):
 
         hook_files = os.path.join(
             os.path.dirname(os.path.realpath(__file__)), 'files')
-        repo_obj = pygit2.Repository(repopath)
+        pygit2.Repository(repopath)
 
         # Install the hook itself
         hook_path = os.path.join(
diff --git a/pagure/hooks/pagure_ticket_hook.py b/pagure/hooks/pagure_ticket_hook.py
index bfad88c..ad604ec 100644
--- a/pagure/hooks/pagure_ticket_hook.py
+++ b/pagure/hooks/pagure_ticket_hook.py
@@ -9,7 +9,6 @@
 """
 
 import os
-import shutil
 
 import flask
 import sqlalchemy as sa
@@ -21,7 +20,7 @@ from sqlalchemy.orm import backref
 
 from pagure.hooks import BaseHook
 from pagure.lib.model import BASE, Project
-from pagure import SESSION, APP
+from pagure import APP
 
 
 class PagureTicketsTable(BASE):
@@ -106,7 +105,7 @@ class PagureTicketHook(BaseHook):
 
         hook_files = os.path.join(
             os.path.dirname(os.path.realpath(__file__)), 'files')
-        repo_obj = pygit2.Repository(repopath)
+        pygit2.Repository(repopath)
 
         # Install the hook itself
         hook_path = os.path.join(
diff --git a/pagure/lib/__init__.py b/pagure/lib/__init__.py
index a02b77c..658ff5d 100644
--- a/pagure/lib/__init__.py
+++ b/pagure/lib/__init__.py
@@ -287,7 +287,8 @@ def add_issue_assignee(session, issue, assignee, user, ticketfolder):
         return 'Issue assigned'
 
 
-def add_pull_request_assignee(session, request, assignee, user, requestfolder):
+def add_pull_request_assignee(
+        session, request, assignee, user, requestfolder):
     ''' Add an assignee to a request, in other words, assigned an issue. '''
     __get_user(session, assignee)
     user_obj = __get_user(session, user)
diff --git a/pagure/lib/git.py b/pagure/lib/git.py
index 156972d..e55b826 100644
--- a/pagure/lib/git.py
+++ b/pagure/lib/git.py
@@ -20,6 +20,8 @@ import tempfile
 import pygit2
 import werkzeug
 
+from sqlalchemy.exc import SQLAlchemyError
+
 import pagure
 import pagure.exceptions
 import pagure.lib
@@ -220,7 +222,6 @@ def clean_git(obj, repo, repofolder, objtype='ticket'):
     index = new_repo.index
 
     # Are we adding files
-    added = False
     if not os.path.exists(file_path):
         shutil.rmtree(newpath)
         return
@@ -229,7 +230,6 @@ def clean_git(obj, repo, repofolder, objtype='ticket'):
     os.unlink(file_path)
 
     diff = new_repo.diff()
-    files = [patch.new_file_path for patch in diff]
 
     # Add the changes to the index
     index.remove(obj.uid)
@@ -522,7 +522,7 @@ def update_request_from_git(
     if assignee:
         pagure.lib.add_pull_request_assignee(
             session, request, assignee.username,
-            user=user.user, ticketfolder=None)
+            user=user.user, requestfolder=None)
 
     for comment in json_data['comments']:
         user = get_user_from_json(session, comment)
@@ -682,7 +682,9 @@ def update_file_in_git(repo, branch, filename, content, message, user):
     files = [patch.new_file_path for patch in diff]
 
     # Add the changes to the index
+    added = False
     for filename in files:
+        added = True
         index.add(filename)
 
     # If not change, return
@@ -727,6 +729,7 @@ def update_file_in_git(repo, branch, filename, content, message, user):
 
 
 def read_output(cmd, abspath, input=None, keepends=False, **kw):
+    """ Read the output from the given command to run """
     if input:
         stdin = subprocess.PIPE
     else:
@@ -893,7 +896,7 @@ def merge_pull_request(
                 session.commit()
             except SQLAlchemyError as err:  # pragma: no cover
                 session.rollback()
-                APP.logger.exception(err)
+                pagure.APP.logger.exception(err)
                 shutil.rmtree(newpath)
                 raise pagure.exceptions.PagureException(
                     'Could not close this pull-request')
@@ -960,7 +963,7 @@ def merge_pull_request(
         session.commit()
     except SQLAlchemyError as err:  # pragma: no cover
         session.rollback()
-        APP.logger.exception(err)
+        pagure.APP.logger.exception(err)
         shutil.rmtree(newpath)
         raise pagure.exceptions.PagureException(
             'Could not update this pull-request in the database')
diff --git a/pagure/lib/notify.py b/pagure/lib/notify.py
index 9330e29..b3b8739 100644
--- a/pagure/lib/notify.py
+++ b/pagure/lib/notify.py
@@ -202,9 +202,10 @@ def send_email(text, subject, to_mail,
         # envelope header.
         msg['To'] = mailto
         salt = pagure.APP.config.get('SALT_EMAIL')
-        m = hashlib.sha512('<%s>%s%s' % (mail_id, salt, mailto))
+        mhash = hashlib.sha512('<%s>%s%s' % (mail_id, salt, mailto))
         msg['Reply-To'] = 'reply+%s@%s' % (
-            m.hexdigest(), pagure.APP.config['DOMAIN_EMAIL_NOTIFICATIONS'])
+            mhash.hexdigest(),
+            pagure.APP.config['DOMAIN_EMAIL_NOTIFICATIONS'])
         try:
             smtp.sendmail(
                 from_email,
diff --git a/pagure/ui/app.py b/pagure/ui/app.py
index c53b7ef..573e56b 100644
--- a/pagure/ui/app.py
+++ b/pagure/ui/app.py
@@ -9,10 +9,8 @@
 """
 
 import flask
-import os
 from math import ceil
 
-import pygit2
 from sqlalchemy.exc import SQLAlchemyError
 
 import pagure.exceptions
@@ -60,8 +58,6 @@ def index():
     user_forks = None
     total_page_repos = None
     total_page_forks = None
-    repos_obj = None
-    forks_obj = None
     username = None
     user_repos_length = None
     user_forks_length = None
diff --git a/pagure/ui/fork.py b/pagure/ui/fork.py
index 8b18e00..cb81346 100644
--- a/pagure/ui/fork.py
+++ b/pagure/ui/fork.py
@@ -10,8 +10,6 @@
 
 import flask
 import os
-import shutil
-import tempfile
 
 import pygit2
 from sqlalchemy.exc import SQLAlchemyError
diff --git a/pagure/ui/groups.py b/pagure/ui/groups.py
index c19d244..09c27c0 100644
--- a/pagure/ui/groups.py
+++ b/pagure/ui/groups.py
@@ -9,7 +9,6 @@
 """
 
 import flask
-import os
 
 from sqlalchemy.exc import SQLAlchemyError
 
@@ -88,11 +87,11 @@ def view_group(group):
             pagure.SESSION.rollback()
             flask.flash(
                 'Could not add user `%s` to group `%s`.' % (
-                    user.user, group.group_name),
+                    username, group.group_name),
                 'error')
             pagure.APP.logger.debug(
                 'Could not add user `%s` to group `%s`.' % (
-                    user.user, group.group_name))
+                    username, group.group_name))
             pagure.APP.logger.exception(err)
 
     member = False
diff --git a/pagure/ui/login.py b/pagure/ui/login.py
index 0167c02..dee3a07 100644
--- a/pagure/ui/login.py
+++ b/pagure/ui/login.py
@@ -22,7 +22,6 @@ import pagure.lib.login
 import pagure.lib.model as model
 import pagure.lib.notify
 from pagure import APP, SESSION
-from pagure.ui.admin import admin_required
 
 # pylint: disable=E1101