s4-python: More cleanups.

9 files changed, 37 insertions, 29 deletions

diff --git a/source4/lib/ldb/tests/python/acl.py b/source4/lib/ldb/tests/python/acl.py
index 4f320ee668..37265ef3d5 100755
--- a/source4/lib/ldb/tests/python/acl.py
+++ b/source4/lib/ldb/tests/python/acl.py
@@ -2,7 +2,6 @@
 # -*- coding: utf-8 -*-
 # This is unit with tests for LDAP access checks
 
-import getopt
 import optparse
 import sys
 import os
@@ -13,9 +12,8 @@ sys.path.append("bin/python")
 
 import samba.getopt as options
 
-from ldb import SCOPE_SUBTREE, SCOPE_ONELEVEL, SCOPE_BASE, LdbError
-from ldb import ERR_NO_SUCH_OBJECT, ERR_INVALID_DN_SYNTAX, ERR_UNWILLING_TO_PERFORM
-from ldb import ERR_INSUFFICIENT_ACCESS_RIGHTS
+from ldb import (
+    SCOPE_BASE, LdbError, ERR_NO_SUCH_OBJECT, ERR_INSUFFICIENT_ACCESS_RIGHTS)
 
 from samba.ndr import ndr_pack, ndr_unpack
 from samba.dcerpc import security
diff --git a/source4/lib/ldb/tests/python/deletetest.py b/source4/lib/ldb/tests/python/deletetest.py
index 55f34fac14..44eb919e09 100755
--- a/source4/lib/ldb/tests/python/deletetest.py
+++ b/source4/lib/ldb/tests/python/deletetest.py
@@ -6,8 +6,9 @@ import sys
 import os
 
 sys.path.append("bin/python")
-sys.path.append("../lib/subunit/python")
-sys.path.append("../lib/testtools")
+import samba
+samba.ensure_external_module("subunit", "subunit")
+samba.ensure_external_module("testtools", "testtools")
 
 import samba.getopt as options
 
diff --git a/source4/scripting/bin/samba_dnsupdate b/source4/scripting/bin/samba_dnsupdate
index 73611c8901..cebfae2871 100755
--- a/source4/scripting/bin/samba_dnsupdate
+++ b/source4/scripting/bin/samba_dnsupdate
@@ -86,12 +86,7 @@ def get_credentials(lp):
         return
     creds = Credentials()
     creds.guess(lp)
-    try:
-        creds.set_machine_account(lp)
-    except:
-        print "Failed to set machine account"
-        raise
-
+    creds.set_machine_account(lp)
     (tmp_fd, ccachename) = tempfile.mkstemp()
     creds.get_named_ccache(lp, ccachename)
 
@@ -200,7 +195,7 @@ def get_subst_vars():
     vars = {}
 
     samdb = SamDB(url=lp.get("sam database"), session_info=system_session(),
-			      lp=lp)
+                  lp=lp)
 
     vars['DNSDOMAIN'] = lp.get('realm').lower()
     vars['HOSTNAME']  = lp.get('netbios name').lower() + "." + vars['DNSDOMAIN']
diff --git a/source4/scripting/bin/upgradeprovision b/source4/scripting/bin/upgradeprovision
index a900728ed4..c33ef6fcb9 100755
--- a/source4/scripting/bin/upgradeprovision
+++ b/source4/scripting/bin/upgradeprovision
@@ -668,6 +668,7 @@ def check_diff_name(newpaths, paths, creds, session, basedn, names, ischema):
                         try:
                             dump_denied_change(dn,att,messageEltFlagToString(msgElt.flags()),current[0][att],reference[0][att])
                         except:
+                            # FIXME: Should catch an explicit exception here
                             dump_denied_change(dn,att,messageEltFlagToString(msgElt.flags()),current[0][att],None)
                     delta.remove(att)
         delta.dn = dn
diff --git a/source4/scripting/python/samba/netcmd/ntacl.py b/source4/scripting/python/samba/netcmd/ntacl.py
index ebb66864d7..ea6bcb6e37 100644
--- a/source4/scripting/python/samba/netcmd/ntacl.py
+++ b/source4/scripting/python/samba/netcmd/ntacl.py
@@ -63,7 +63,8 @@ class cmd_acl_set(Command):
         creds = credopts.get_credentials(lp)
         creds.set_kerberos_state(DONT_USE_KERBEROS)
         try:
-            ldb = Ldb(path, session_info=system_session(), credentials=creds,lp=lp)
+            ldb = Ldb(path, session_info=system_session(), credentials=creds,
+                      lp=lp)
         except:
             # XXX: Should catch a particular exception type
             raise CommandError("Unable to read domain SID from configuration files")
diff --git a/source4/scripting/python/samba/ntacls.py b/source4/scripting/python/samba/ntacls.py
index edcd643b2a..16e9463f09 100644
--- a/source4/scripting/python/samba/ntacls.py
+++ b/source4/scripting/python/samba/ntacls.py
@@ -41,29 +41,37 @@ def checkset_backend(lp,backend,eadbfile):
 def getntacl(lp, file, backend=None, eadbfile=None):
     checkset_backend(lp, backend, eadbfile)
     eadbname = lp.get("posix:eadb")
-    if eadbname != None and eadbname != "" :
+    if eadbname is not None and eadbname != "":
         try:
-            attribute = samba.xattr_tdb.wrap_getxattr(eadbname,file,xattr.XATTR_NTACL_NAME)
+            attribute = samba.xattr_tdb.wrap_getxattr(eadbname, file, 
+                xattr.XATTR_NTACL_NAME)
         except:
+            # FIXME: Don't catch all exceptions, just those related to opening 
+            # xattrdb
             print "Fail to open %s" % eadbname
-            attribute = samba.xattr_native.wrap_getxattr(file,xattr.XATTR_NTACL_NAME)
+            attribute = samba.xattr_native.wrap_getxattr(file,
+                xattr.XATTR_NTACL_NAME)
     else:
-        attribute = samba.xattr_native.wrap_getxattr(file,xattr.XATTR_NTACL_NAME)
+        attribute = samba.xattr_native.wrap_getxattr(file,
+            xattr.XATTR_NTACL_NAME)
     ntacl = ndr_unpack(xattr.NTACL,attribute)
     return ntacl
 
 def setntacl(lp, file, sddl, domsid, backend=None, eadbfile=None):
-    checkset_backend(lp,backend,eadbfile)
+    checkset_backend(lp, backend, eadbfile)
     ntacl=xattr.NTACL()
     ntacl.version = 1
     sid=security.dom_sid(domsid)
     sd = security.descriptor.from_sddl(sddl, sid)
     ntacl.info = sd
     eadbname = lp.get("posix:eadb")
-    if eadbname != None  and eadbname != "":
+    if eadbname is not None and eadbname != "":
         try:
-            samba.xattr_tdb.wrap_setxattr(eadbname,file,xattr.XATTR_NTACL_NAME,ndr_pack(ntacl))
+            samba.xattr_tdb.wrap_setxattr(eadbname,
+                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))
     else:
diff --git a/source4/scripting/python/samba/provision.py b/source4/scripting/python/samba/provision.py
index 71c04cfef9..abbcf52210 100644
--- a/source4/scripting/python/samba/provision.py
+++ b/source4/scripting/python/samba/provision.py
@@ -269,7 +269,8 @@ def setup_ldb(ldb, ldif_path, subst_vars):
     except:
         ldb.transaction_cancel()
         raise
-    ldb.transaction_commit()
+    else:
+        ldb.transaction_commit()
 
 
 def provision_paths_from_lp(lp, dnsdomain):
@@ -550,12 +551,11 @@ def setup_samdb_partitions(samdb_path, setup_path, message, lp, session_info,
 
         message("Setting up sam.ldb rootDSE")
         setup_samdb_rootdse(samdb, setup_path, names)
-
     except:
         samdb.transaction_cancel()
         raise
-
-    samdb.transaction_commit()
+    else:
+        samdb.transaction_commit()
 
         
 def secretsdb_self_join(secretsdb, domain, 
diff --git a/source4/scripting/python/samba/samdb.py b/source4/scripting/python/samba/samdb.py
index 0dcea72ad7..2f33c31c63 100644
--- a/source4/scripting/python/samba/samdb.py
+++ b/source4/scripting/python/samba/samdb.py
@@ -150,7 +150,8 @@ pwdLastSet: 0
         except:
             self.transaction_cancel()
             raise
-        self.transaction_commit()
+        else:
+            self.transaction_commit()
 
     def setpassword(self, filter, password, force_change_at_next_login=False):
         """Sets the password for a user
@@ -188,7 +189,8 @@ userPassword:: %s
         except:
             self.transaction_cancel()
             raise
-        self.transaction_commit()
+        else:
+            self.transaction_commit()
 
     def setexpiry(self, filter, expiry_seconds, no_expiry_req=False):
         """Sets the account expiry for a user
@@ -227,7 +229,8 @@ accountExpires: %u
         except:
             self.transaction_cancel()
             raise
-        self.transaction_commit()
+        else:
+            self.transaction_commit()
 
     def set_domain_sid(self, sid):
         """Change the domain SID used by this LDB.
diff --git a/source4/setup/provision b/source4/setup/provision
index d0d35f807e..117433de4d 100755
--- a/source4/setup/provision
+++ b/source4/setup/provision
@@ -219,7 +219,8 @@ elif opts.use_xattrs == "auto":
 			"O:S-1-5-32G:S-1-5-32", "S-1-5-32", "native")
 		eadb = False
 	except:
-		if lp.get("posix:eadb") == None:
+		# XXX: Should catch a specific exception here
+		if lp.get("posix:eadb") is None:
 			message("Notice: you are not root or your system do not support xattr, tdb backend for attributes has been selected")
 			message(" if you intend to use this provision in production you'd better rerun the script as root on a system supporting xattr")
 	file.close()