Clean up some problems discovered with pylint and pychecker

Much of this is formatting to make pylint happy but it also fixes some
real bugs.

17 files changed, 99 insertions, 107 deletions

diff --git a/ipalib/errors.py b/ipalib/errors.py
index 82905b391..caecdc382 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -992,6 +992,22 @@ class RemoteRetrieveError(ExecutionError):
     errno = 4016
     format = _('%(reason)s')
 
+class SameGroupError(ExecutionError):
+    """
+    **4017** Raised when adding a group as a member of itself
+
+    For example:
+
+    >>> raise SameGroupError()
+    Traceback (most recent call last):
+      ...
+    SameGroupError: A group may not be added as a member of itself
+
+    """
+
+    errno = 4017
+    format = _('A group may not be added as a member of itself')
+
 class BuiltinError(ExecutionError):
     """
     **4100** Base class for builtin execution errors (*4100 - 4199*).
diff --git a/ipapython/entity.py b/ipapython/entity.py
index 580cbd00b..1274f8a13 100644
--- a/ipapython/entity.py
+++ b/ipapython/entity.py
@@ -15,10 +15,6 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 #
 
-import ldap
-import ldif
-import re
-import cStringIO
 import copy
 
 import ipapython.ipautil
diff --git a/ipapython/ipasslfile.py b/ipapython/ipasslfile.py
index 5525ab9b5..5f4ea2a7b 100644
--- a/ipapython/ipasslfile.py
+++ b/ipapython/ipasslfile.py
@@ -15,6 +15,10 @@
 # the reference counting and SharedSocketClient provides an constructor
 # and close() method that call incref() and decref() correctly.
 
+import socket
+import errno
+from httplib import UnimplementedFileMode
+
 class SharedSocket:
     def __init__(self, sock):
         self.sock = sock
@@ -91,13 +95,13 @@ class SSLFile(SharedSocketClient):
                 break
             L.append(s)
             avail += len(s)
-        all = "".join(L)
+        alldata = "".join(L)
         if size is None:
             self._buf = ''
-            return all
+            return alldata
         else:
-            self._buf = all[size:]
-            return all[:size]
+            self._buf = alldata[size:]
+            return alldata[:size]
 
     def readline(self):
         L = [self._buf]
@@ -114,25 +118,25 @@ class SSLFile(SharedSocketClient):
             # loop exited because there is no more data
             return "".join(L)
         else:
-            all = "".join(L)
+            alldata = "".join(L)
             # XXX could do enough bookkeeping not to do a 2nd search
-            i = all.find("\n") + 1
-            line = all[:i]
-            self._buf = all[i:]
+            i = alldata.find("\n") + 1
+            line = alldata[:i]
+            self._buf = alldata[i:]
             return line
 
     def readlines(self, sizehint=0):
         total = 0
-        list = []
+        inlist = []
         while True:
             line = self.readline()
             if not line:
                 break
-            list.append(line)
+            inlist.append(line)
             total += len(line)
             if sizehint and total >= sizehint:
                 break
-        return list
+        return inlist
 
     def fileno(self):
         return self._sock.fileno()
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index df721a518..2fa26fef8 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -57,7 +57,7 @@ def get_domain_name():
     try:
         config.init_config()
         domain_name = config.config.get_domain()
-    except Exception, e:
+    except Exception:
         return None
 
     return domain_name
@@ -362,13 +362,13 @@ class GeneralizedTimeZone(datetime.tzinfo):
         if self.houroffset < 0:
             self.minoffset *= -1
 
-    def utcoffset(self, dt):
+    def utcoffset(self):
         return datetime.timedelta(hours=self.houroffset, minutes=self.minoffset)
 
-    def dst(self, dt):
+    def dst(self):
         return datetime.timedelta(0)
 
-    def tzname(self, dt):
+    def tzname(self):
         return self.name
 
 
@@ -748,7 +748,7 @@ class AttributeValueCompleter:
             return self.default_value.get(self.lhs, None)
         elif default_value_type is FunctionType:
             return self.default_value(self.lhs)
-        elif default_value_type is StringsType:
+        elif default_value_type is StringType:
             return self.default_value
         else:
             return None
@@ -759,7 +759,7 @@ class AttributeValueCompleter:
         else:
             self.completions = self.lhs_names
 
-    def complete(self, text, state):
+    def complete(self, state):
         self.line_buffer= readline.get_line_buffer()
         self.parse_input()
         if not self.lhs_complete:
@@ -905,7 +905,7 @@ class ItemCompleter:
         else:
             self.completions = self.items
 
-    def complete(self, text, state):
+    def complete(self, state):
         self.line_buffer= readline.get_line_buffer()
         if state == 0:
             beg = readline.get_begidx()
diff --git a/ipapython/nsslib.py b/ipapython/nsslib.py
index 1b678305b..05b45c14a 100644
--- a/ipapython/nsslib.py
+++ b/ipapython/nsslib.py
@@ -20,7 +20,6 @@
 import httplib
 import getpass
 import socket
-import errno
 
 from nss.error import NSPRError
 import nss.io as io
diff --git a/ipapython/sysrestore.py b/ipapython/sysrestore.py
index 503f38b24..78c9b6283 100644
--- a/ipapython/sysrestore.py
+++ b/ipapython/sysrestore.py
@@ -25,7 +25,6 @@
 
 import os
 import os.path
-import errno
 import shutil
 import logging
 import ConfigParser
@@ -110,13 +109,13 @@ class FileStore:
             logging.debug("  -> Not backing up - '%s' doesn't exist", path)
             return
 
-        (reldir, file) = os.path.split(path)
+        (reldir, backupfile) = os.path.split(path)
 
         filename = ""
         for i in range(8):
             h = "%02x" % self.random.randint(0,255)
             filename += h
-        filename += "-"+file
+        filename += "-"+backupfile
 
         backup_path = os.path.join(self._path, filename)
         if os.path.exists(backup_path):
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index cadab10e3..f5a704614 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -17,12 +17,9 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 #
 
-import string
 import tempfile
-import shutil
 import os
 import pwd
-import socket
 import logging
 
 import installutils
@@ -73,7 +70,7 @@ class BindInstance(service.Service):
 
         tmp = ip_address.split(".")
         tmp.reverse()
-        
+
         self.reverse_host = tmp.pop(0)
         self.reverse_subnet = ".".join(tmp)
 
@@ -232,7 +229,6 @@ class BindInstance(service.Service):
     def uninstall(self):
         running = self.restore_state("running")
         enabled = self.restore_state("enabled")
-        domain = self.restore_state("domain")
 
         if not running is None:
             self.stop()
diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index a8a1a774c..5ade47160 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -575,7 +575,7 @@ class CAInstance(service.Service):
     def __restart_instance(self):
         try:
             self.restart()
-        except Exception, e:
+        except Exception:
             # TODO: roll back here?
             logging.critical("Failed to restart the certificate server. See the installation log for details.")
 
@@ -625,8 +625,6 @@ class CAInstance(service.Service):
         params['submit'] = 'submit'
         params['requestNotes'] = ''
         params = urllib.urlencode(params)
-        headers = {"Content-type": "application/x-www-form-urlencoded",
-                   "Accept": "text/plain"}
 
         # Now issue the RA certificate.
         args = [
@@ -826,8 +824,6 @@ class CAInstance(service.Service):
         os.chown(self.ra_agent_pwd, pent.pw_uid, pent.pw_gid)
 
     def __setup_sign_profile(self):
-        caconfig = "/var/lib/pki-ca/conf/CS.cfg"
-
         # Tell the profile to automatically issue certs for RAs
         installutils.set_directive('/var/lib/pki-ca/profiles/ca/caJarSigningCert.cfg', 'auth.instance_id', 'raCertAuth', quotes=False, separator='=')
 
diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index 76eb8dd95..f66959bc1 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -26,7 +26,6 @@ import sys
 import os
 import re
 import time
-import stat
 
 from ipapython import ipautil
 
@@ -36,7 +35,7 @@ import certs
 import ldap
 from ipaserver import ipaldap
 from ipaserver.install import ldapupdate
-from ipalib import util
+from ipalib import util, errors
 
 SERVER_ROOT_64 = "/usr/lib64/dirsrv"
 SERVER_ROOT_32 = "/usr/lib/dirsrv"
@@ -319,7 +318,7 @@ class DsInstance(service.Service):
             dsdb.create_from_pkcs12(self.pkcs12_info[0], self.pkcs12_info[1])
             server_certs = dsdb.find_server_certs()
             if len(server_certs) == 0:
-                raise RuntimeError("Could not find a suitable server cert in import in %s" % pkcs12_info[0])
+                raise RuntimeError("Could not find a suitable server cert in import in %s" % self.pkcs12_info[0])
 
             # We only handle one server cert
             nickname = server_certs[0][0]
@@ -453,7 +452,7 @@ class DsInstance(service.Service):
         status = True
         try:
             certdb.load_cacert(cacert_fname)
-        except ipalib.CalledProcessError, e:
+        except errors.CalledProcessError, e:
             logging.critical("Error importing CA cert file named [%s]: %s" %
                              (cacert_fname, str(e)))
             status = False
diff --git a/ipaserver/install/httpinstance.py b/ipaserver/install/httpinstance.py
index ba9de6138..b6483fb3d 100644
--- a/ipaserver/install/httpinstance.py
+++ b/ipaserver/install/httpinstance.py
@@ -19,13 +19,9 @@
 
 import os
 import os.path
-import subprocess
-import string
 import tempfile
 import logging
 import pwd
-import fileinput
-import sys
 import shutil
 
 import service
@@ -105,8 +101,8 @@ class HTTPInstance(service.Service):
         if selinux:
             try:
                 # returns e.g. "httpd_can_network_connect --> off"
-                (stdout, stderr) = ipautils.run(["/usr/sbin/getsebool",
-                                                 "httpd_can_network_connect"])
+                (stdout, stderr) = ipautil.run(["/usr/sbin/getsebool",
+                                                "httpd_can_network_connect"])
                 self.backup_state("httpd_can_network_connect", stdout.split()[2])
             except:
                 pass
@@ -170,7 +166,7 @@ class HTTPInstance(service.Service):
             db.create_from_pkcs12(self.pkcs12_info[0], self.pkcs12_info[1], passwd="")
             server_certs = db.find_server_certs()
             if len(server_certs) == 0:
-                raise RuntimeError("Could not find a suitable server cert in import in %s" % pkcs12_info[0])
+                raise RuntimeError("Could not find a suitable server cert in import in %s" % self.pkcs12_info[0])
 
             db.create_password_conf()
             # We only handle one server cert
diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index c12023719..a41176cc7 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -25,7 +25,6 @@ import os
 import re
 import fileinput
 import sys
-import time
 import struct
 import fcntl
 
@@ -217,28 +216,28 @@ def set_directive(filename, directive, value, quotes=True, separator=' '):
     """
     valueset = False
     fd = open(filename)
-    file = []
+    newfile = []
     for line in fd:
         if directive in line:
             valueset = True
             if quotes:
-                file.append('%s%s"%s"\n' % (directive, separator, value))
+                newfile.append('%s%s"%s"\n' % (directive, separator, value))
             else:
-                file.append('%s%s%s\n' % (directive, separator, value))
+                newfile.append('%s%s%s\n' % (directive, separator, value))
         else:
-            file.append(line)
+            newfile.append(line)
     fd.close()
     if not valueset:
         if quotes:
-            file.append('%s%s"%s"\n' % (directive, separator, value))
+            newfile.append('%s%s"%s"\n' % (directive, separator, value))
         else:
-            file.append('%s%s%s\n' % (directive, separator, value))
+            newfile.append('%s%s%s\n' % (directive, separator, value))
 
     fd = open(filename, "w")
-    fd.write("".join(file))
+    fd.write("".join(newfile))
     fd.close()
 
-def get_directive(filename, directive, strip_quotes=True, separator=' '):
+def get_directive(filename, directive, separator=' '):
     """
     A rather inefficient way to get a configuration directive.
     """
diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index 1c3489725..a6caa81eb 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -17,8 +17,6 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 #
 
-import subprocess
-import string
 import shutil
 import logging
 import fileinput
@@ -27,7 +25,6 @@ import sys
 import os
 import pwd
 import socket
-import shutil
 
 import service
 import installutils
@@ -46,7 +43,6 @@ from pyasn1.type import univ, namedtype
 import pyasn1.codec.ber.encoder
 import pyasn1.codec.ber.decoder
 import struct
-import base64
 
 KRBMKEY_DENY_ACI = """
 (targetattr = "krbMKey")(version 3.0; acl "No external access"; deny (all) userdn != "ldap:///uid=kdc,cn=sysaccounts,cn=etc,$SUFFIX";)
@@ -225,7 +221,7 @@ class KrbInstance(service.Service):
             msgid = self.conn.search("cn=mapping,cn=sasl,cn=config", ldap.SCOPE_ONELEVEL, "(objectclass=nsSaslMapping)")
             res = self.conn.result(msgid)
             for r in res[1]:
-                mid = self.conn.delete_s(r[0])
+                self.conn.delete_s(r[0])
         #except LDAPError, e:
         #    logging.critical("Error during SASL mapping removal: %s" % str(e))
         except Exception, e:
@@ -301,7 +297,7 @@ class KrbInstance(service.Service):
     def __write_stash_from_ds(self):
         try:
             entry = self.conn.getEntry("cn=%s, cn=kerberos, %s" % (self.realm, self.suffix), ldap.SCOPE_SUBTREE)
-        except errors.NotFound:
+        except errors.NotFound, e:
             logging.critical("Could not find master key in DS")
             raise e
 
diff --git a/ipaserver/install/ntpinstance.py b/ipaserver/install/ntpinstance.py
index 3f76551e8..2c53b5611 100644
--- a/ipaserver/install/ntpinstance.py
+++ b/ipaserver/install/ntpinstance.py
@@ -17,7 +17,6 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 #
 
-import shutil
 import logging
 
 import service
diff --git a/ipaserver/install/replication.py b/ipaserver/install/replication.py
index 653ec3c4e..242352975 100644
--- a/ipaserver/install/replication.py
+++ b/ipaserver/install/replication.py
@@ -27,11 +27,11 @@ from ipalib import util
 from ipalib import errors
 
 DIRMAN_CN = "cn=directory manager"
-CACERT="/usr/share/ipa/html/ca.crt"
+CACERT = "/usr/share/ipa/html/ca.crt"
 # the default container used by AD for user entries
-WIN_USER_CONTAINER="cn=Users"
+WIN_USER_CONTAINER = "cn=Users"
 # the default container used by IPA for user entries
-IPA_USER_CONTAINER="cn=users,cn=accounts"
+IPA_USER_CONTAINER = "cn=users,cn=accounts"
 PORT = 636
 TIMEOUT = 120
 
@@ -351,9 +351,11 @@ class ReplicationManager:
     def check_repl_init(self, conn, agmtdn):
         done = False
         hasError = 0
-        attrlist = ['cn', 'nsds5BeginReplicaRefresh', 'nsds5replicaUpdateInProgress',
-					'nsds5ReplicaLastInitStatus', 'nsds5ReplicaLastInitStart',
-				    'nsds5ReplicaLastInitEnd']
+        attrlist = ['cn', 'nsds5BeginReplicaRefresh',
+                    'nsds5replicaUpdateInProgress',
+                    'nsds5ReplicaLastInitStatus',
+                    'nsds5ReplicaLastInitStart',
+                    'nsds5ReplicaLastInitEnd']
         entry = conn.getEntry(agmtdn, ldap.SCOPE_BASE, "(objectclass=*)", attrlist)
         if not entry:
             print "Error reading status from agreement", agmtdn
diff --git a/ipaserver/ipautil.py b/ipaserver/ipautil.py
index 6422fe5a6..7042c1ce2 100644
--- a/ipaserver/ipautil.py
+++ b/ipaserver/ipautil.py
@@ -17,11 +17,13 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 #
 
-import string
 import xmlrpclib
 import re
 
 def realm_to_suffix(realm_name):
+    """
+    Convert a kerberos realm into the IPA suffix.
+    """
     s = realm_name.split(".")
     terms = ["dc=" + x.lower() for x in s]
     return ",".join(terms)
@@ -37,32 +39,32 @@ class CIDict(dict):
     If you extend UserDict, isinstance(foo, dict) returns false.
     """
 
-    def __init__(self,default=None):
+    def __init__(self, default=None):
         super(CIDict, self).__init__()
         self._keys = {}
         self.update(default or {})
 
-    def __getitem__(self,key):
-        return super(CIDict,self).__getitem__(string.lower(key))
+    def __getitem__(self, key):
+        return super(CIDict, self).__getitem__(key.lower())
 
-    def __setitem__(self,key,value):
-        lower_key = string.lower(key)
+    def __setitem__(self, key, value):
+        lower_key = key.lower()
         self._keys[lower_key] = key
-        return super(CIDict,self).__setitem__(string.lower(key),value)
+        return super(CIDict, self).__setitem__(lower_key, value)
 
-    def __delitem__(self,key):
-        lower_key = string.lower(key)
+    def __delitem__(self, key):
+        lower_key = key.lower()
         del self._keys[lower_key]
-        return super(CIDict,self).__delitem__(string.lower(key))
+        return super(CIDict, self).__delitem__(key.lower())
 
-    def update(self,dict):
+    def update(self, dict):
         for key in dict.keys():
             self[key] = dict[key]
 
-    def has_key(self,key):
-        return super(CIDict, self).has_key(string.lower(key))
+    def has_key(self, key):
+        return super(CIDict, self).has_key(key.lower())
 
-    def get(self,key,failobj=None):
+    def get(self, key, failobj=None):
         try:
             return self[key]
         except KeyError:
@@ -74,7 +76,7 @@ class CIDict(dict):
     def items(self):
         result = []
         for k in self._keys.values():
-            result.append((k,self[k]))
+            result.append((k, self[k]))
         return result
 
     def copy(self):
@@ -89,7 +91,7 @@ class CIDict(dict):
     def iterkeys(self):
         return self.copy().iterkeys()
 
-    def setdefault(self,key,value=None):
+    def setdefault(self, key, value=None):
         try:
             return self[key]
         except KeyError:
@@ -107,11 +109,11 @@ class CIDict(dict):
             raise
 
     def popitem(self):
-        (lower_key,value) = super(CIDict,self).popitem()
+        (lower_key, value) = super(CIDict, self).popitem()
         key = self._keys[lower_key]
         del self._keys[lower_key]
 
-        return (key,value)
+        return (key, value)
 
 
 #
@@ -127,10 +129,10 @@ SAFE_STRING_PATTERN = '(^(\000|\n|\r| |:|<)|[\000\n\r\200-\377]+|[ ]+$)'
 safe_string_re = re.compile(SAFE_STRING_PATTERN)
 
 def needs_base64(s):
-  """
-  returns 1 if s has to be base-64 encoded because of special chars
-  """
-  return not safe_string_re.search(s) is None
+    """
+    returns 1 if s has to be base-64 encoded because of special chars
+    """
+    return not safe_string_re.search(s) is None
 
 
 def wrap_binary_data(data):
@@ -148,7 +150,7 @@ def wrap_binary_data(data):
         return retval
     elif isinstance(data, dict):
         retval = {}
-        for (k,v) in data.iteritems():
+        for (k, v) in data.iteritems():
             retval[k] = wrap_binary_data(v)
         return retval
     else:
@@ -170,7 +172,7 @@ def unwrap_binary_data(data):
         return retval
     elif isinstance(data, dict):
         retval = {}
-        for (k,v) in data.iteritems():
+        for (k, v) in data.iteritems():
             retval[k] = unwrap_binary_data(v)
         return retval
     else:
@@ -181,21 +183,21 @@ def get_gsserror(e):
        in python 2.5, deal with it."""
 
     try:
-       primary = e[0]
-       secondary = e[1]
-    except:
-       primary = e[0][0]
-       secondary = e[0][1]
+        primary = e[0]
+        secondary = e[1]
+    except Exception:
+        primary = e[0][0]
+        secondary = e[0][1]
 
     return (primary[0], secondary[0])
 
 def utf8_encode_value(value):
-    if isinstance(value,unicode):
+    if isinstance(value, unicode):
         return value.encode('utf-8')
     return value
 
 def utf8_encode_values(values):
-    if isinstance(values,list) or isinstance(values,tuple):
+    if isinstance(values, list) or isinstance(values, tuple):
         return map(utf8_encode_value, values)
     else:
         return utf8_encode_value(values)
diff --git a/ipaserver/plugins/ldap2.py b/ipaserver/plugins/ldap2.py
index 4df288dfc..9587cbe2e 100644
--- a/ipaserver/plugins/ldap2.py
+++ b/ipaserver/plugins/ldap2.py
@@ -28,16 +28,12 @@ Backend plugin for LDAP.
 
 import copy
 import os
-import re
 import socket
 import string
 
-import krbV
 import ldap as _ldap
 import ldap.filter as _ldap_filter
 import ldap.sasl as _ldap_sasl
-from ldap.controls import LDAPControl
-from ldap.ldapobject import SimpleLDAPObject
 # for backward compatibility
 from ldap.functions import explode_dn
 
diff --git a/ipaserver/servercore.py b/ipaserver/servercore.py
index 74014c758..ed490312b 100644
--- a/ipaserver/servercore.py
+++ b/ipaserver/servercore.py
@@ -18,11 +18,8 @@
 #
 
 import ldap
-import string
-import re
 from ipalib.request import context
 from ipaserver import ipaldap
-import ipautil
 from ipalib import errors
 from ipalib import api