s4-python: Fix formatting.

7 files changed, 176 insertions, 133 deletions

diff --git a/source4/scripting/python/samba/__init__.py b/source4/scripting/python/samba/__init__.py
index dcb80a7e5d..d870879a1e 100644
--- a/source4/scripting/python/samba/__init__.py
+++ b/source4/scripting/python/samba/__init__.py
@@ -154,7 +154,11 @@ class Ldb(_Ldb):
                 raise
 
     def erase_except_schema_controlled(self):
-        """Erase this ldb, removing all records, except those that are controlled by Samba4's schema."""
+        """Erase this ldb.
+        
+        :note: Removes all records, except those that are controlled by
+            Samba4's schema.
+        """
 
         basedn = ""
 
@@ -173,8 +177,7 @@ class Ldb(_Ldb):
                     raise
 
         res = self.search(basedn, ldb.SCOPE_SUBTREE,
-                          "(&(|(objectclass=*)(distinguishedName=*))(!(distinguishedName=@BASEINFO)))",
-                          [], controls=["show_deleted:0"])
+            "(&(|(objectclass=*)(distinguishedName=*))(!(distinguishedName=@BASEINFO)))", [], controls=["show_deleted:0"])
         assert len(res) == 0
 
         # delete the specials
@@ -297,20 +300,20 @@ class Ldb(_Ldb):
         dsdb.dsdb_set_ntds_invocation_id(self, invocation_id)
 
     def get_invocation_id(self):
-        "Get the invocation_id id"
+        """Get the invocation_id id"""
         return dsdb.samdb_ntds_invocation_id(self)
 
     def get_ntds_GUID(self):
-        "Get the NTDS objectGUID"
+        """Get the NTDS objectGUID"""
         return dsdb.samdb_ntds_objectGUID(self)
 
     def server_site_name(self):
-        "Get the server site name"
+        """Get the server site name"""
         return dsdb.samdb_server_site_name(self)
 
 
 def substitute_var(text, values):
-    """substitute strings of the form ${NAME} in str, replacing
+    """Substitute strings of the form ${NAME} in str, replacing
     with substitutions from subobj.
 
     :param text: Text in which to subsitute.
@@ -360,13 +363,15 @@ def setup_file(template, fname, subst_vars=None):
     :param fname: Path of the file to create.
     :param subst_vars: Substitution variables.
     """
-    f = fname
-
-    if os.path.exists(f):
-        os.unlink(f)
+    if os.path.exists(fname):
+        os.unlink(fname)
 
     data = read_and_sub_file(template, subst_vars)
-    open(f, 'w').write(data)
+    f = open(fname, 'w')
+    try:
+        f.write(data)
+    finally:
+        f.close()
 
 
 def valid_netbios_name(name):
diff --git a/source4/scripting/python/samba/idmap.py b/source4/scripting/python/samba/idmap.py
index c0b20bd257..1cb33fd0a9 100644
--- a/source4/scripting/python/samba/idmap.py
+++ b/source4/scripting/python/samba/idmap.py
@@ -57,28 +57,27 @@ class IDmapDB(samba.Ldb):
 
         :return xid can that be used for SID/unixid mapping
         """
-        res=self.search(expression="dn=CN=CONFIG",base="", scope=ldb.SCOPE_SUBTREE)
-        id=res[0].get("xidNumber")
-        flag=ldb.FLAG_MOD_REPLACE
-        if id == None:
-            id=res[0].get("lowerBound")
+        res = self.search(expression="dn=CN=CONFIG", base="", 
+                          scope=ldb.SCOPE_SUBTREE)
+        id = res[0].get("xidNumber")
+        flag = ldb.FLAG_MOD_REPLACE
+        if id is None:
+            id = res[0].get("lowerBound")
             flag = ldb.FLAG_MOD_ADD
         newid = int(str(id)) + 1
         msg = ldb.Message()
-        msg.dn = ldb.Dn(self,"CN=CONFIG")
-        msg["xidNumber"] = ldb.MessageElement(str(newid),flag,"xidNumber")
+        msg.dn = ldb.Dn(self, "CN=CONFIG")
+        msg["xidNumber"] = ldb.MessageElement(str(newid), flag, "xidNumber")
         self.modify(msg)
-
         return id
 
-
     def setup_name_mapping(self, sid, type, unixid=None):
         """Setup a mapping between a sam name and a unix name.
 
         :param sid: SID of the NT-side of the mapping.
         :param unixname: Unix id to map to, if none supplied the next one will be selected
         """
-        if unixid == None:
+        if unixid is None:
             unixid = self.increment_xid()
         type_string = ""
         if type == self.TYPE_UID:
diff --git a/source4/scripting/python/samba/ntacls.py b/source4/scripting/python/samba/ntacls.py
index 478a5125bf..cfdb2621c4 100644
--- a/source4/scripting/python/samba/ntacls.py
+++ b/source4/scripting/python/samba/ntacls.py
@@ -30,7 +30,7 @@ class XattrBackendError(Exception):
 def checkset_backend(lp, backend, eadbfile):
     if backend is not None:
         if backend == "native":
-            lp.set("posix:eadb","")
+            lp.set("posix:eadb", "")
         elif backend == "tdb":
             if eadbfile != None:
                 lp.set("posix:eadb", eadbfile)
@@ -56,9 +56,10 @@ def getntacl(lp, file, backend=None, eadbfile=None):
     else:
         attribute = samba.xattr_native.wrap_getxattr(file,
             xattr.XATTR_NTACL_NAME)
-    ntacl = ndr_unpack(xattr.NTACL,attribute)
+    ntacl = ndr_unpack(xattr.NTACL, attribute)
     return ntacl
 
+
 def setntacl(lp, file, sddl, domsid, backend=None, eadbfile=None):
     checkset_backend(lp, backend, eadbfile)
     ntacl = xattr.NTACL()
@@ -70,17 +71,21 @@ def setntacl(lp, file, sddl, domsid, backend=None, eadbfile=None):
     if eadbname is not None and eadbname != "":
         try:
             samba.xattr_tdb.wrap_setxattr(eadbname,
-                file,xattr.XATTR_NTACL_NAME,ndr_pack(ntacl))
+                file, xattr.XATTR_NTACL_NAME, ndr_pack(ntacl))
         except:
             # FIXME: Don't catch all exceptions, just those related to opening 
             # xattrdb
-            print "Fail to open %s"%eadbname
-            samba.xattr_native.wrap_setxattr(file,xattr.XATTR_NTACL_NAME,ndr_pack(ntacl))
+            print "Fail to open %s" % eadbname
+            samba.xattr_native.wrap_setxattr(file, xattr.XATTR_NTACL_NAME, 
+                ndr_pack(ntacl))
     else:
-        samba.xattr_native.wrap_setxattr(file,xattr.XATTR_NTACL_NAME,ndr_pack(ntacl))
+        samba.xattr_native.wrap_setxattr(file, xattr.XATTR_NTACL_NAME,
+                ndr_pack(ntacl))
+
 
 def ldapmask2filemask(ldm):
-    """Takes the access mask of a DS ACE and transform them in a File ACE mask"""
+    """Takes the access mask of a DS ACE and transform them in a File ACE mask.
+    """
     RIGHT_DS_CREATE_CHILD     = 0x00000001
     RIGHT_DS_DELETE_CHILD     = 0x00000002
     RIGHT_DS_LIST_CONTENTS    = 0x00000004
@@ -141,7 +146,7 @@ def dsacl2fsacl(dssddl, domsid):
     for files. It's used for Policy object provision
     """
     sid = security.dom_sid(domsid)
-    ref = security.descriptor.from_sddl(dssddl,sid)
+    ref = security.descriptor.from_sddl(dssddl, sid)
     fdescr = security.descriptor()
     fdescr.owner_sid = ref.owner_sid
     fdescr.group_sid = ref.group_sid
diff --git a/source4/scripting/python/samba/provision.py b/source4/scripting/python/samba/provision.py
index 089d4e3e01..c8e082a876 100644
--- a/source4/scripting/python/samba/provision.py
+++ b/source4/scripting/python/samba/provision.py
@@ -47,12 +47,17 @@ from samba.dsdb import DS_DOMAIN_FUNCTION_2003, DS_DC_FUNCTION_2008
 from samba.dcerpc import security
 from samba.dcerpc.misc import SEC_CHAN_BDC, SEC_CHAN_WKSTA
 from samba.idmap import IDmapDB
+from samba.ms_display_specifiers import read_ms_ldif
 from samba.ntacls import setntacl, dsacl2fsacl
 from samba.ndr import ndr_pack,ndr_unpack
+from samba.provisionbackend import (
+    ExistingBackend,
+    FDSBackend,
+    LDBBackend,
+    OpenLDAPBackend,
+    )
 from samba.schema import Schema
 from samba.samdb import SamDB
-from ms_display_specifiers import read_ms_ldif
-from samba.provisionbackend import LDBBackend, ExistingBackend, FDSBackend, OpenLDAPBackend
 
 __docformat__ = "restructuredText"
 
diff --git a/source4/scripting/python/samba/provisionbackend.py b/source4/scripting/python/samba/provisionbackend.py
index 8d035ab670..076ceb969f 100644
--- a/source4/scripting/python/samba/provisionbackend.py
+++ b/source4/scripting/python/samba/provisionbackend.py
@@ -70,16 +70,20 @@ class ProvisionBackend(object):
         self.ldap_backend_type = backend_type
 
     def init(self):
-        pass
+        """Initialize the backend."""
+        raise NotImplementedError(self.init)
 
     def start(self):
-        pass
+        """Start the backend."""
+        raise NotImplementedError(self.start)
 
     def shutdown(self):
-        pass
+        """Shutdown the backend."""
+        raise NotImplementedError(self.shutdown)
 
     def post_setup(self):
-        pass
+        """Post setup."""
+        raise NotImplementedError(self.post_setup)
 
 
 class LDBBackend(ProvisionBackend):
@@ -114,7 +118,8 @@ class ExistingBackend(ProvisionBackend):
         # into the long-term database later in the script.
         self.secrets_credentials = self.credentials
 
-        self.ldap_backend_type = "openldap" # For now, assume existing backends at least emulate OpenLDAP
+         # For now, assume existing backends at least emulate OpenLDAP
+        self.ldap_backend_type = "openldap"
 
 
 class LDAPBackend(ProvisionBackend):
@@ -143,7 +148,7 @@ class LDAPBackend(ProvisionBackend):
         self.ldap_backend_extra_port = ldap_backend_extra_port
         self.ldap_dryrun_mode = ldap_dryrun_mode
 
-        self.ldapi_uri = "ldapi://" + urllib.quote(os.path.join(self.ldapdir, "ldapi"), safe="")
+        self.ldapi_uri = "ldapi://%s" % urllib.quote(os.path.join(self.ldapdir, "ldapi"), safe="")
 
         if not os.path.exists(self.ldapdir):
             os.mkdir(self.ldapdir)
@@ -171,7 +176,8 @@ class LDAPBackend(ProvisionBackend):
             # XXX: We should never be catching all Ldb errors
             pass
 
-        # Try to print helpful messages when the user has not specified the path to slapd
+        # Try to print helpful messages when the user has not specified the
+        # path to slapd
         if self.slapd_path is None:
             raise ProvisioningError("Warning: LDAP-Backend must be setup with path to slapd, e.g. --slapd-path=\"/usr/local/libexec/slapd\"!")
         if not os.path.exists(self.slapd_path):
@@ -195,13 +201,13 @@ class LDAPBackend(ProvisionBackend):
 
         self.credentials = Credentials()
         self.credentials.guess(self.lp)
-        #Kerberos to an ldapi:// backend makes no sense
+        # Kerberos to an ldapi:// backend makes no sense
         self.credentials.set_kerberos_state(DONT_USE_KERBEROS)
         self.credentials.set_password(self.ldapadminpass)
 
         self.secrets_credentials = Credentials()
         self.secrets_credentials.guess(self.lp)
-        #Kerberos to an ldapi:// backend makes no sense
+        # Kerberos to an ldapi:// backend makes no sense
         self.secrets_credentials.set_kerberos_state(DONT_USE_KERBEROS)
         self.secrets_credentials.set_username("samba-admin")
         self.secrets_credentials.set_password(self.ldapadminpass)
@@ -214,7 +220,11 @@ class LDAPBackend(ProvisionBackend):
     def start(self):
         from samba.provision import ProvisioningError
         self.slapd_command_escaped = "\'" + "\' \'".join(self.slapd_command) + "\'"
-        open(os.path.join(self.ldapdir, "ldap_backend_startup.sh"), 'w').write("#!/bin/sh\n" + self.slapd_command_escaped + "\n")
+        f = open(os.path.join(self.ldapdir, "ldap_backend_startup.sh"), 'w')
+        try:
+            f.write("#!/bin/sh\n" + self.slapd_command_escaped + "\n")
+        finally:
+            f.close()
 
         # Now start the slapd, so we can provision onto it.  We keep the
         # subprocess context around, to kill this off at the successful
@@ -239,13 +249,13 @@ class LDAPBackend(ProvisionBackend):
                     self.message("Could not connect to slapd started with: %s" %  "\'" + "\' \'".join(self.slapd_provision_command) + "\'")
                     raise ProvisioningError("slapd never accepted a connection within 15 seconds of starting")
 
-        self.message("Could not start slapd with: %s" %  "\'" + "\' \'".join(self.slapd_provision_command) + "\'")
+        self.message("Could not start slapd with: %s" % "\'" + "\' \'".join(self.slapd_provision_command) + "\'")
         raise ProvisioningError("slapd died before we could make a connection to it")
 
     def shutdown(self):
         # if an LDAP backend is in use, terminate slapd after final provision and check its proper termination
         if self.slapd.poll() is None:
-            #Kill the slapd
+            # Kill the slapd
             if hasattr(self.slapd, "terminate"):
                 self.slapd.terminate()
             else:
@@ -253,7 +263,7 @@ class LDAPBackend(ProvisionBackend):
                 import signal
                 os.kill(self.slapd.pid, signal.SIGTERM)
     
-            #and now wait for it to die
+            # and now wait for it to die
             self.slapd.communicate()
 
 
@@ -264,7 +274,6 @@ class OpenLDAPBackend(LDAPBackend):
             schema=None, hostname=None, ldapadminpass=None, slapd_path=None,
             ldap_backend_extra_port=None, ldap_dryrun_mode=False,
             ol_mmr_urls=None, nosync=False):
-
         super(OpenLDAPBackend, self).__init__( backend_type=backend_type,
                 paths=paths, setup_path=setup_path, lp=lp,
                 credentials=credentials, names=names, message=message,
@@ -364,25 +373,28 @@ class OpenLDAPBackend(LDAPBackend):
                                                             "LDAPSERVER" : url })
                     rid=serverid*10
                     rid=rid+1
-                    mmr_syncrepl_schema_config += read_and_sub_file(self.setup_path("mmr_syncrepl.conf"),
-                                                                {  "RID" : str(rid),
-                                                                   "MMRDN": self.names.schemadn,
-                                                                   "LDAPSERVER" : url,
-                                                                   "MMR_PASSWORD": mmr_pass})
-                
-                    rid=rid+1
-                    mmr_syncrepl_config_config += read_and_sub_file(self.setup_path("mmr_syncrepl.conf"),
-                                                                {  "RID" : str(rid),
-                                                                   "MMRDN": self.names.configdn,
-                                                                   "LDAPSERVER" : url,
-                                                                   "MMR_PASSWORD": mmr_pass})
+                    mmr_syncrepl_schema_config += read_and_sub_file(
+                            self.setup_path("mmr_syncrepl.conf"),
+                            {  "RID" : str(rid),
+                               "MMRDN": self.names.schemadn,
+                               "LDAPSERVER" : url,
+                               "MMR_PASSWORD": mmr_pass})
+    
+                    rid = rid+1
+                    mmr_syncrepl_config_config += read_and_sub_file(
+                        self.setup_path("mmr_syncrepl.conf"), {
+                            "RID" : str(rid),
+                            "MMRDN": self.names.configdn,
+                            "LDAPSERVER" : url,
+                            "MMR_PASSWORD": mmr_pass})
                 
-                    rid=rid+1
-                    mmr_syncrepl_user_config += read_and_sub_file(self.setup_path("mmr_syncrepl.conf"),
-                                                              {  "RID" : str(rid),
-                                                                 "MMRDN": self.names.domaindn,
-                                                                 "LDAPSERVER" : url,
-                                                                 "MMR_PASSWORD": mmr_pass })
+                    rid = rid+1
+                    mmr_syncrepl_user_config += read_and_sub_file(
+                        self.setup_path("mmr_syncrepl.conf"), {
+                            "RID" : str(rid),
+                            "MMRDN": self.names.domaindn,
+                            "LDAPSERVER" : url,
+                            "MMR_PASSWORD": mmr_pass })
         # OpenLDAP cn=config initialisation
         olc_syncrepl_config = ""
         olc_mmr_config = "" 
@@ -392,23 +404,24 @@ class OpenLDAPBackend(LDAPBackend):
             serverid=0
             olc_serverids_config = ""
             olc_syncrepl_seed_config = ""
-            olc_mmr_config += read_and_sub_file(self.setup_path("olc_mmr.conf"),{})
-            rid=500
+            olc_mmr_config += read_and_sub_file(
+                self.setup_path("olc_mmr.conf"), {})
+            rid = 500
             for url in url_list:
-                serverid=serverid+1
-                olc_serverids_config += read_and_sub_file(self.setup_path("olc_serverid.conf"),
-                                                      { "SERVERID" : str(serverid),
-                                                        "LDAPSERVER" : url })
+                serverid = serverid + 1
+                olc_serverids_config += read_and_sub_file(
+                    self.setup_path("olc_serverid.conf"), {
+                        "SERVERID" : str(serverid), "LDAPSERVER" : url })
             
-                rid=rid+1
-                olc_syncrepl_config += read_and_sub_file(self.setup_path("olc_syncrepl.conf"),
-                                                     {  "RID" : str(rid),
-                                                        "LDAPSERVER" : url,
-                                                        "MMR_PASSWORD": mmr_pass})
+                rid = rid + 1
+                olc_syncrepl_config += read_and_sub_file(
+                    self.setup_path("olc_syncrepl.conf"), {
+                        "RID" : str(rid), "LDAPSERVER" : url,
+                        "MMR_PASSWORD": mmr_pass})
             
-                olc_syncrepl_seed_config += read_and_sub_file(self.setup_path("olc_syncrepl_seed.conf"),
-                                                          {  "RID" : str(rid),
-                                                             "LDAPSERVER" : url})
+                olc_syncrepl_seed_config += read_and_sub_file(
+                    self.setup_path("olc_syncrepl_seed.conf"), {
+                        "RID" : str(rid), "LDAPSERVER" : url})
                 
             setup_file(self.setup_path("olc_seed.ldif"), self.olcseedldif,
                        {"OLC_SERVER_ID_CONF": olc_serverids_config,
@@ -438,23 +451,23 @@ class OpenLDAPBackend(LDAPBackend):
         self.setup_db_config(os.path.join(self.ldapdir, "db", "user"))
         self.setup_db_config(os.path.join(self.ldapdir, "db", "config"))
         self.setup_db_config(os.path.join(self.ldapdir, "db", "schema"))
-    
-        if not os.path.exists(os.path.join(self.ldapdir, "db", "samba",  "cn=samba")):
-            os.makedirs(os.path.join(self.ldapdir, "db", "samba",  "cn=samba"), 0700)
+
+        if not os.path.exists(os.path.join(self.ldapdir, "db", "samba", "cn=samba")):
+            os.makedirs(os.path.join(self.ldapdir, "db", "samba", "cn=samba"), 0700)
         
         setup_file(self.setup_path("cn=samba.ldif"), 
-                   os.path.join(self.ldapdir, "db", "samba",  "cn=samba.ldif"),
+                   os.path.join(self.ldapdir, "db", "samba", "cn=samba.ldif"),
                    { "UUID": str(uuid.uuid4()), 
                      "LDAPTIME": timestring(int(time.time()))} )
         setup_file(self.setup_path("cn=samba-admin.ldif"), 
-                   os.path.join(self.ldapdir, "db", "samba",  "cn=samba", "cn=samba-admin.ldif"),
+                   os.path.join(self.ldapdir, "db", "samba", "cn=samba", "cn=samba-admin.ldif"),
                    {"LDAPADMINPASS_B64": b64encode(self.ldapadminpass),
                     "UUID": str(uuid.uuid4()), 
                     "LDAPTIME": timestring(int(time.time()))} )
     
         if self.ol_mmr_urls is not None:
             setup_file(self.setup_path("cn=replicator.ldif"),
-                       os.path.join(self.ldapdir, "db", "samba",  "cn=samba", "cn=replicator.ldif"),
+                       os.path.join(self.ldapdir, "db", "samba", "cn=samba", "cn=replicator.ldif"),
                        {"MMR_PASSWORD_B64": b64encode(mmr_pass),
                         "UUID": str(uuid.uuid4()),
                         "LDAPTIME": timestring(int(time.time()))} )
@@ -463,9 +476,15 @@ class OpenLDAPBackend(LDAPBackend):
         mapping = "schema-map-openldap-2.3"
         backend_schema = "backend-schema.schema"
 
-        backend_schema_data = self.schema.ldb.convert_schema_to_openldap("openldap", open(self.setup_path(mapping), 'r').read())
+        f = open(self.setup_path(mapping), 'r')
+        backend_schema_data = self.schema.ldb.convert_schema_to_openldap(
+                "openldap", f.read())
         assert backend_schema_data is not None
-        open(os.path.join(self.ldapdir, backend_schema), 'w').write(backend_schema_data)
+        f = open(os.path.join(self.ldapdir, backend_schema), 'w')
+        try:
+            f.write(backend_schema_data)
+        finally:
+            f.close()
 
         # now we generate the needed strings to start slapd automatically,
         # first ldapi_uri...
@@ -481,18 +500,16 @@ class OpenLDAPBackend(LDAPBackend):
         else:
             server_port_string = ""
 
-        # Prepare the 'result' information - the commands to return in particular
-        self.slapd_provision_command = [self.slapd_path]
-
-        self.slapd_provision_command.append("-F" + self.olcdir)
-
-        self.slapd_provision_command.append("-h")
+        # Prepare the 'result' information - the commands to return in
+        # particular
+        self.slapd_provision_command = [self.slapd_path, "-F" + self.olcdir, 
+            "-h"]
 
-        # copy this command so we have two version, one with -d0 and only ldapi, and one with all the listen commands
+        # copy this command so we have two version, one with -d0 and only
+        # ldapi, and one with all the listen commands
         self.slapd_command = list(self.slapd_provision_command)
     
-        self.slapd_provision_command.append(self.ldapi_uri)
-        self.slapd_provision_command.append("-d0")
+        self.slapd_provision_command.extend([self.ldapi_uri, "-d0"])
 
         uris = self.ldapi_uri
         if server_port_string is not "":
@@ -500,7 +517,8 @@ class OpenLDAPBackend(LDAPBackend):
 
         self.slapd_command.append(uris)
 
-        # Set the username - done here because Fedora DS still uses the admin DN and simple bind
+        # Set the username - done here because Fedora DS still uses the admin
+        # DN and simple bind
         self.credentials.set_username("samba-admin")
     
         # If we were just looking for crashes up to this point, it's a
@@ -513,7 +531,9 @@ class OpenLDAPBackend(LDAPBackend):
         if not os.path.isdir(self.olcdir):
             os.makedirs(self.olcdir, 0770)
 
-            retcode = subprocess.call([self.slapd_path, "-Ttest", "-n", "0", "-f", self.slapdconf, "-F", self.olcdir], close_fds=True, shell=False)
+            retcode = subprocess.call([self.slapd_path, "-Ttest", "-n", "0",
+                "-f", self.slapdconf, "-F", self.olcdir], close_fds=True,
+                shell=False)
 
             if retcode != 0:
                 raise ProvisioningError("conversion from slapd.conf to cn=config failed")
@@ -624,13 +644,13 @@ class FDSBackend(LDAPBackend):
         for attr in lnkattr.keys():
             if lnkattr[attr] is not None:
                 refint_config += read_and_sub_file(self.setup_path("fedorads-refint-add.ldif"),
-                                                 { "ARG_NUMBER" : str(argnum) ,
-                                                   "LINK_ATTR" : attr })
+                         { "ARG_NUMBER" : str(argnum),
+                           "LINK_ATTR" : attr })
                 memberof_config += read_and_sub_file(self.setup_path("fedorads-linked-attributes.ldif"),
-                                                 { "MEMBER_ATTR" : attr ,
-                                                   "MEMBEROF_ATTR" : lnkattr[attr] })
-                index_config += read_and_sub_file(self.setup_path("fedorads-index.ldif"),
-                                                 { "ATTR" : attr })
+                         { "MEMBER_ATTR" : attr,
+                           "MEMBEROF_ATTR" : lnkattr[attr] })
+                index_config += read_and_sub_file(
+                    self.setup_path("fedorads-index.ldif"), { "ATTR" : attr })
                 argnum += 1
 
         open(self.refint_ldif, 'w').write(refint_config)
@@ -645,8 +665,8 @@ class FDSBackend(LDAPBackend):
             if attr == "objectGUID":
                 attr = "nsUniqueId"
 
-            index_config += read_and_sub_file(self.setup_path("fedorads-index.ldif"),
-                                             { "ATTR" : attr })
+            index_config += read_and_sub_file(
+                self.setup_path("fedorads-index.ldif"), { "ATTR" : attr })
 
         open(self.index_ldif, 'w').write(index_config)
 
@@ -661,20 +681,28 @@ class FDSBackend(LDAPBackend):
         # Build a schema file in Fedora DS format
         backend_schema_data = self.schema.ldb.convert_schema_to_openldap("fedora-ds", open(self.setup_path(mapping), 'r').read())
         assert backend_schema_data is not None
-        open(os.path.join(self.ldapdir, backend_schema), 'w').write(backend_schema_data)
+        f = open(os.path.join(self.ldapdir, backend_schema), 'w')
+        try:
+            f.write(backend_schema_data)
+        finally:
+            f.close()
 
         self.credentials.set_bind_dn(self.names.ldapmanagerdn)
 
         # Destory the target directory, or else setup-ds.pl will complain
-        fedora_ds_dir = os.path.join(self.ldapdir, "slapd-" + self.ldap_instance)
+        fedora_ds_dir = os.path.join(self.ldapdir,
+            "slapd-" + self.ldap_instance)
         shutil.rmtree(fedora_ds_dir, True)
 
-        self.slapd_provision_command = [self.slapd_path, "-D", fedora_ds_dir, "-i", self.slapd_pid]
-        #In the 'provision' command line, stay in the foreground so we can easily kill it
+        self.slapd_provision_command = [self.slapd_path, "-D", fedora_ds_dir,
+                "-i", self.slapd_pid]
+        # In the 'provision' command line, stay in the foreground so we can
+        # easily kill it
         self.slapd_provision_command.append("-d0")
 
         #the command for the final run is the normal script
-        self.slapd_command = [os.path.join(self.ldapdir, "slapd-" + self.ldap_instance, "start-slapd")]
+        self.slapd_command = [os.path.join(self.ldapdir,
+            "slapd-" + self.ldap_instance, "start-slapd")]
 
         # If we were just looking for crashes up to this point, it's a
         # good time to exit before we realise we don't have Fedora DS on
@@ -703,17 +731,14 @@ class FDSBackend(LDAPBackend):
     def post_setup(self):
         ldapi_db = Ldb(self.ldapi_uri, credentials=self.credentials)
 
-        # configure in-directory access control on Fedora DS via the aci attribute (over a direct ldapi:// socket)
+        # configure in-directory access control on Fedora DS via the aci
+        # attribute (over a direct ldapi:// socket)
         aci = """(targetattr = "*") (version 3.0;acl "full access to all by samba-admin";allow (all)(userdn = "ldap:///CN=samba-admin,%s");)""" % self.sambadn
         
         m = ldb.Message()
         m["aci"] = ldb.MessageElement([aci], ldb.FLAG_MOD_REPLACE, "aci")
 
-        m.dn = ldb.Dn(ldapi_db, self.names.domaindn)
-        ldapi_db.modify(m)
-            
-        m.dn = ldb.Dn(ldapi_db, self.names.configdn)
-        ldapi_db.modify(m)
-            
-        m.dn = ldb.Dn(ldapi_db, self.names.schemadn)
-        ldapi_db.modify(m)
+        for dnstring in (self.names.domaindn, self.names.configdn,
+                         self.names.schemadn):
+            m.dn = ldb.Dn(ldapi_db, dnstring)
+            ldapi_db.modify(m)
diff --git a/source4/scripting/python/samba/schema.py b/source4/scripting/python/samba/schema.py
index 562fe3891d..fc4f131932 100644
--- a/source4/scripting/python/samba/schema.py
+++ b/source4/scripting/python/samba/schema.py
@@ -23,12 +23,11 @@
 """Functions for setting up a Samba Schema."""
 
 from base64 import b64encode
-from ms_schema import read_ms_schema
-from samba.dcerpc import security
 from samba import read_and_sub_file, substitute_var, check_all_substituted
-from samba.samdb import SamDB
-from samba import Ldb
+from samba.dcerpc import security
+from samba.ms_schema import read_ms_schema
 from samba.ndr import ndr_pack
+from samba.samdb import SamDB
 from ldb import SCOPE_SUBTREE, SCOPE_ONELEVEL
 import os
 
diff --git a/source4/scripting/python/samba/upgradehelpers.py b/source4/scripting/python/samba/upgradehelpers.py
index 50439efbc7..ce1b3e3736 100755
--- a/source4/scripting/python/samba/upgradehelpers.py
+++ b/source4/scripting/python/samba/upgradehelpers.py
@@ -27,13 +27,14 @@ import string
 import re
 import shutil
 
-from samba import Ldb
-from samba.dsdb import DS_DOMAIN_FUNCTION_2000
 from ldb import SCOPE_SUBTREE, SCOPE_ONELEVEL, SCOPE_BASE
 import ldb
+
+from samba import Ldb
+from samba.dcerpc import misc, security
+from samba.dsdb import DS_DOMAIN_FUNCTION_2000
 from samba.provision import (ProvisionNames, provision_paths_from_lp,
     FILL_FULL, provision, ProvisioningError)
-from samba.dcerpc import misc, security
 from samba.ndr import ndr_unpack
 
 
@@ -106,7 +107,7 @@ def find_provision_key_parameters(param, credentials, session_info, paths,
     configdn = str(names.configdn)
     names.schemadn = current[0]["schemaNamingContext"]
     if not (ldb.Dn(samdb, basedn) == (ldb.Dn(samdb, current[0]["defaultNamingContext"][0]))):
-        raise ProvisioningError(("basedn in %s (%s) and from %s (%s) is not the same ..." % (paths.samdb, str(current[0]["defaultNamingContext"][0]), paths.smbconf, basedn)))
+        raise ProvisioningError("basedn in %s (%s) and from %s (%s) is not the same ..." % (paths.samdb, str(current[0]["defaultNamingContext"][0]), paths.smbconf, basedn))
 
     names.domaindn=current[0]["defaultNamingContext"]
     names.rootdn=current[0]["rootDomainNamingContext"]
@@ -120,7 +121,8 @@ def find_provision_key_parameters(param, credentials, session_info, paths,
         base="OU=Domain Controllers,"+basedn, scope=SCOPE_ONELEVEL, attrs=["dNSHostName"])
     names.hostname = str(res4[0]["dNSHostName"]).replace("."+names.dnsdomain,"")
 
-    server_res = samdb.search(expression="serverReference=%s"%res4[0].dn, attrs=[], base=configdn)
+    server_res = samdb.search(expression="serverReference=%s" % res4[0].dn,
+            attrs=[], base=configdn)
     names.serverdn = server_res[0].dn
 
     # invocation id/objectguid
@@ -136,18 +138,21 @@ def find_provision_key_parameters(param, credentials, session_info, paths,
                 "objectSid","msDS-Behavior-Version" ])
     names.domainguid = str(ndr_unpack( misc.GUID,res6[0]["objectGUID"][0]))
     names.domainsid = ndr_unpack( security.dom_sid,res6[0]["objectSid"][0])
-    if res6[0].get("msDS-Behavior-Version") == None or int(res6[0]["msDS-Behavior-Version"][0]) < DS_DOMAIN_FUNCTION_2000:
+    if (res6[0].get("msDS-Behavior-Version") is None or
+        int(res6[0]["msDS-Behavior-Version"][0]) < DS_DOMAIN_FUNCTION_2000):
         names.domainlevel = DS_DOMAIN_FUNCTION_2000
     else:
         names.domainlevel = int(res6[0]["msDS-Behavior-Version"][0])
 
     # policy guid
-    res7 = samdb.search(expression="(displayName=Default Domain Policy)",base="CN=Policies,CN=System,"+basedn, \
-                            scope=SCOPE_ONELEVEL, attrs=["cn","displayName"])
+    res7 = samdb.search(expression="(displayName=Default Domain Policy)",
+            base="CN=Policies,CN=System,"+basedn, scope=SCOPE_ONELEVEL,
+            attrs=["cn","displayName"])
     names.policyid = str(res7[0]["cn"]).replace("{","").replace("}","")
     # dc policy guid
-    res8 = samdb.search(expression="(displayName=Default Domain Controllers Policy)",base="CN=Policies,CN=System,"+basedn, \
-                            scope=SCOPE_ONELEVEL, attrs=["cn","displayName"])
+    res8 = samdb.search(expression="(displayName=Default Domain Controllers Policy)",
+            base="CN=Policies,CN=System,"+basedn, scope=SCOPE_ONELEVEL,
+            attrs=["cn","displayName"])
     if len(res8) == 1:
         names.policyid_dc = str(res8[0]["cn"]).replace("{","").replace("}","")
     else: