Muehlenhoff has submitted this change and it was merged.

Change subject: Various cleanups
......................................................................


Various cleanups

Massive whitespace cleanups and fixes various issues found in initial review
by Filippo.

Change-Id: I680695b7ad1939aa0e5cb90be81f6de949f79cbc
---
M master/debdeploy
M master/debdeploy_conf.py
M master/debdeploy_joblog.py
M master/debdeploy_updatespec.py
M minion/debdeploy-minion.py
M minion/debdeploy_restart.py
M tests/test-jobdb
M tests/test-pkgdb
8 files changed, 91 insertions(+), 144 deletions(-)

Approvals:
  Muehlenhoff: Verified; Looks good to me, approved



diff --git a/master/debdeploy b/master/debdeploy
index cdac55b..62a7677 100755
--- a/master/debdeploy
+++ b/master/debdeploy
@@ -32,7 +32,7 @@
     install_errors = []
     restart_cnt = {}
 
-    for i in conf.server_groups[opt.serverlist]: 
+    for i in conf.server_groups[opt.serverlist]:
 
         if rollback_mode:
             if not joblogdb.does_job_exist(opt.updatefile, i):
@@ -47,7 +47,7 @@
                 print opt.updatefile, "hasn't been deployed yet for this 
server group (identified by grain", i, ")"
                 sys.exit(0)
 
-        if rollback_mode:        
+        if rollback_mode:
             jid = joblogdb.get_rollbackid(opt.updatefile, i)
         else:
             jid = joblogdb.get_jobid(opt.updatefile, i)
@@ -65,13 +65,13 @@
             sys.exit(0)
 
         amount_of_hosts += len(res.keys())
-            
-        for host in res.keys():
+
+        for host in res:
             added = res[host]['return']['additions']
             removed = res[host]['return']['removals']
             updated = res[host]['return']['updated']
             restart = res[host]['return']['restart']
-            
+
             print host + ":"
             if len(added) > 0:
                 print "  Added packages:", host['return']['additions']
@@ -80,7 +80,7 @@
                         add_cnt[added_pkg] = 1
                     else:
                         add_cnt[added_pkg] += 1
-                        
+
             elif len(removed) > 0:
                 print "  Removed packages:", host['return']['removals']
                 for removed_pkg in host['return']['removals']:
@@ -91,7 +91,7 @@
 
             elif len(updated.keys()) > 0:
                 print "  Updated packages:"
-                for k in updated.keys():
+                for k in updated:
                     print "    " + k + ": " + updated[k][0] + " -> " + 
updated[k][1]
 
                     updated_idx = k + ": " + updated[k][0] + " -> " + 
updated[k][1]
@@ -111,7 +111,7 @@
                             restart_cnt[process] = 1
                         else:
                             restart_cnt[process] += 1
-                        
+
             # Detect installation errors and collect them to display an error 
list
             # across all Salt grains
             # apt doesn't provide more fine-grained error reporting other than
@@ -122,12 +122,12 @@
                     install_errors.append(host + ": The dpkg status database 
is locked, possibly a conflicting package installation")
                 else:
                     install_errors.append(host + ": Unknown installation 
error")
-                
+
             if opt.verbose:
                 print
                 indented = ['  ' + l for l in 
res[host]['return']['aptlog'].splitlines()]
                 print "\n".join(indented)
-                        
+
     print 
     print 
     if rollback_mode:
@@ -142,14 +142,14 @@
         print "No packages were added"
     else:
         print "Added packages:"
-        for pkg in add_cnt.keys():
+        for pkg in add_cnt:
             print pkg, "on", add_cnt[pkg], "hosts"
 
     if len(remove_cnt.keys()) == 0:
         print "No packages were removed"
     else:
         print "Removed packages:"
-        for pkg in remove_cnt.keys():
+        for pkg in remove_cnt:
             print pkg, "on", remove_cnt[pkg], "hosts"
 
     if len(update_cnt.keys()) == 0:
@@ -167,8 +167,8 @@
         print "Restarts needed:"
         for process in sorted(restart_cnt.keys()):
             print process, "on", restart_cnt[process], "hosts"
-            
-    print 
+
+    print
     print "Error summary:"
 
     if len(install_errors) == 0:
@@ -186,8 +186,8 @@
     grains      : Apply the update on this list of Salt grains (list of 
strings)
     update_file : Filename of update specification (string)
     '''
-    
-    print "Rolling out", source, ":", 
+
+    print "Rolling out", source, ":",
     if update_type == "tool":
         print "Non-daemon update, no service restart needed"
     elif update_type == "daemon-direct":
@@ -208,8 +208,8 @@
         sys.exit(1)
     elif update_type == "library":
         print "Library update, several services might need to be restarted"
-    
-    for i in grains: 
+
+    for i in grains:
         if joblogdb.has_been_rolled_back(update_file, i):
             print update_file, "was already deployed and rolled back for this 
server group (identified by grain", i, "), if you want to redeploy you need to 
assign a different name"
             sys.exit(1)
@@ -223,7 +223,7 @@
         jid = client.cmd_async(i, 'debdeploy-minion.deploy', [source, 
update_type, update.fixes], expr_form='grain')
         logging.info("Initiated rollout for " + source + " on grain " + i + 
"(salt job id: " + str(jid) + ")")
         joblogdb.add_job(update_file, i, jid)
-            
+
 
 def restart(grains, processes):
     '''
@@ -241,15 +241,15 @@
     print "Restarting services. Use --verbose to also display non-failing 
restarts."
     for i in grains:
         r = client.cmd(i, 'debdeploy-minion.restart_service', [processes], 
expr_form='grain')
-        for host in r.keys():
+        for host in r:
             c += 1
             if opt.verbose:
                 print host + ":"
-            for process in r[host].keys():
+            for process in r[host]:
                 c_success.setdefault(process, 0)
                 c_failed.setdefault(process, 0)
                 c_stopped.setdefault(process, 0)
-                            
+
                 if r[host][process] == 0:
                     c_success[process] += 1
                     if opt.verbose:
@@ -270,14 +270,14 @@
     if opt.verbose:
         print
     print "Restart summary:"
-    for proc in c_success.keys():
+    for proc in c_success:
         print proc, "successfully restarted on", c_success[proc], "out of", c, 
"hosts."
 
-    for proc in c_failed.keys():
+    for proc in c_failed:
         if c_failed[proc] > 0:
             print proc, "failed to restart on", c_failed[proc], "out of", c, 
"hosts."
 
-    for proc in c_stopped.keys():
+    for proc in c_stopped:
         if c_stopped[proc] > 0:
             print proc, "wasn't running on on", c_stopped[proc], "out of", c, 
"hosts."
 
@@ -289,8 +289,8 @@
     grains      : Rollback an update on this list of Salt grains (list of 
strings)
     update_file : Filename of update specification (string)
     '''
-    
-    for servergroup in grains: 
+
+    for servergroup in grains:
         if not joblogdb.does_job_exist(update_file, servergroup):
             print "No software deployment for " + update_file + " exists."
             sys.exit(1)
@@ -301,14 +301,14 @@
             else:
                 jid = joblogdb.get_jobid(update_file, servergroup)
             print "Initiated rollback for JID", jid
-                
+
         rid = client.cmd_async(servergroup, 'debdeploy-minion.rollback', 
[jid], expr_form='grain')
 
         joblogdb.mark_as_rolled_back(jid, rid)
 
 
-            
-            
+
+
 client = salt.client.LocalClient()
 conf = DebDeployConfig("/etc/debdeploy.conf")
 joblogdb = DebDeployJobLog("/var/lib/debdeploy/jobdb.sqlite")
@@ -341,7 +341,7 @@
     sys.exit(1)
 
 command = args[0]
-    
+
 if command in ("deploy", "status-deploy", "rollback", "status-rollback", 
"restart"):
     if not opt.serverlist:
         op.error("You need to provide a server list (-s)")
@@ -361,31 +361,30 @@
         op.error("You need to provide a hostname (-h)")
 
 
-        
 if command == "deploy":
     update = DebDeployUpdateSpec(opt.updatefile, conf.supported_distros)
     deploy_update(update.source, update.update_type, 
conf.server_groups[opt.serverlist], opt.updatefile)
 
 elif command == "restart":
     restart(conf.server_groups[opt.serverlist], opt.program)
-        
+
 elif command == "status-deploy":
-    display_status()    
+    display_status()
 
 elif command == "status-rollback":
-    display_status(rollback_mode=True)    
+    display_status(rollback_mode=True)
 
 elif command == "rollback":
     rollback(conf.server_groups[opt.serverlist], opt.updatefile)
-              
+
 # elif command == "pkgdb-source":
 #     jid = client.cmd(opt.host, 'debdeploy.return_pkgs')
-    
+
 #     if opt.serverlist == "all":
 #         ret = client.cmd("*", 'debdeploy-minion.deploy', [update.source, 
update.fixes])
 #     else:
 #     # there can be multiple grains specified in a server list
-#         for i in conf.server_groups[opt.serverlist]: 
+#         for i in conf.server_groups[opt.serverlist]:
 #             if joblogdb.has_been_rolled_back(opt.updatefile, i):
 #                 print opt.updatefile, "was already deployed and rolled back 
for this server group (identified by grain", i, "), if you want to redeploy you 
need to assign a different name"
 #                 sys.exit(1)
@@ -395,22 +394,9 @@
 #                 sys.exit(1)
 #             jid = client.cmd_async(i, 'debdeploy-minion.deploy', 
[update.source, update.fixes], expr_form='grain')
 #             joblogdb.add_job(opt.updatefile, i, jid)
-        
+
 sys.exit(0)
 
 # Local variables:
 # mode: python
 # End:
-
-
-
-
-
-
-
-
-
-
-
-
-
diff --git a/master/debdeploy_conf.py b/master/debdeploy_conf.py
index c0f6708..076df8e 100644
--- a/master/debdeploy_conf.py
+++ b/master/debdeploy_conf.py
@@ -13,16 +13,12 @@
     '''
     supported_distros = []
     server_groups = {}
-    
+
     def __init__(self, configfile):
         config = ConfigParser.ConfigParser()
-        try:
-            if len(config.read(configfile)) == 0:
-                print "/etc/debdeploy.conf doesn't exist, you need to create 
it."
-                print "See 
/usr/share/doc/debdeploy-master/examples/debdeploy.conf"
-                sys.exit(1)
-        except:
-            print "Failed to open", configfile
+        if len(config.read(configfile)) == 0:
+            print "/etc/debdeploy.conf doesn't exist, you need to create it."
+            print "See /usr/share/doc/debdeploy-master/examples/debdeploy.conf"
             sys.exit(1)
 
         if not config.has_section("distros") or not 
config.has_option("distros", "supported"):
@@ -47,14 +43,7 @@
                     else:
                         print "Malformed server list, at least one grain must 
be specified for the server group", i
                         sys.exit(1)
-                
+
 # Local variables:
 # mode: python
 # End:
-
-
-
-
-
-
-
diff --git a/master/debdeploy_joblog.py b/master/debdeploy_joblog.py
index 34369a0..f515be0 100644
--- a/master/debdeploy_joblog.py
+++ b/master/debdeploy_joblog.py
@@ -13,13 +13,13 @@
 
     def __init__(self, dbfilename):
         self.sqlite_dbfilename = dbfilename
-        
+
         if not os.path.exists(self.sqlite_dbfilename):
             conn = sqlite3.connect(self.sqlite_dbfilename)
 
             with conn:
                 conn.execute('CREATE TABLE updates (updatespec text, grain 
text, jobid text, rollbackid text)')
-                
+
     def add_job(self, yamlfile, grain, jid):
         '''
         This function records a software deployment in the job database.
@@ -32,7 +32,7 @@
         v = (str(yamlfile), str(grain), str(jid), "")
         with conn:
             conn.execute('INSERT INTO updates VALUES (?, ?, ?, ?)', v)
-        
+
     def does_job_exist(self, yamlfile, grain):
         '''
         This boolean function returns whether a software update has been 
deployed yet.
@@ -94,14 +94,14 @@
         grain = group of servers identified by a grain (string)
         '''
         conn = sqlite3.connect(self.sqlite_dbfilename)
-        
+
         with conn:
             r = conn.execute("SELECT rollbackid FROM updates WHERE 
updatespec=? and grain=?", (yamlfile,grain,)).fetchall()
             if not r:
                 return None
             else:
                 return r[0][0]
-    
+
     def mark_as_rolled_back(self, jid, rid):
         '''
         This function records that a rollback has been issued via Salt.
@@ -119,5 +119,3 @@
 # Local variables:
 # mode: python
 # End:
-
-
diff --git a/master/debdeploy_updatespec.py b/master/debdeploy_updatespec.py
index dbc71a4..bdcccc6 100644
--- a/master/debdeploy_updatespec.py
+++ b/master/debdeploy_updatespec.py
@@ -9,13 +9,13 @@
     Each update is described in a YAML file, see docs/readme.txt for the data
     format.
     '''
-    
+
     source = ""
     comment = ""
     update_type = ""
     fixes = {}
     legit_type = ['tool', 'daemon-direct', 'daemon-disrupt', 'daemon-cluster', 
'reboot', 'reboot-cluster', 'library']
-    
+
     def __init__(self, updatespec, supported_distros):
         '''
         Parse an update spec file.
@@ -23,30 +23,30 @@
         updatespec        : Filename of the update spec file (string)
         supported_distros : These are the distro codenames for which a fixed 
version can be provided (list of strings)
         '''
-        
+
         try:
-            stream = open(updatespec, "r")
-        except:
+            with open(updatespec, "r") as stream:
+                updatefile = yaml.load(stream)
+
+        except IOError:
             print "Error: Could not open", updatespec
             sys.exit(1)
 
-        updatefile = yaml.load(stream)
-            
         if not updatefile.has_key("source"):
             print "Invalid YAML file, you need to specify the source package 
using the 'source' stanza, see the annotated example file for details"
             sys.exit(1)
         else:
             self.source = updatefile["source"]
-            
+
         if not updatefile.has_key("update_type"):
             print "Invalid YAML file, you need to specify the type of update 
using the 'update_type' stanza, see the annotated example file for details"
             sys.exit(1)
         else:
-            if updatefile["update_type"] not in legit_type:
+            if updatefile["update_type"] not in self.legit_type:
                 print "Invalid YAML file, invalid 'update_type'"
                 sys.exit(1)
             self.update_type = updatefile["update_type"]
-            
+
         if updatefile.has_key("comment"):
             self.comment = updatefile["comment"]
 
@@ -54,7 +54,7 @@
             print "Invalid YAML file, you need to specify at least one fixed 
version using the 'fixes' stanza, see the annotated example file for details"
             sys.exit(1)
         else:
-            for i in updatefile["fixes"].keys():
+            for i in updatefile["fixes"]:
                 if supported_distros.count(i) >= 1:
                     self.fixes[i] = updatefile["fixes"].get(i)
                 else:
diff --git a/minion/debdeploy-minion.py b/minion/debdeploy-minion.py
index d5b1c8d..78ecb47 100644
--- a/minion/debdeploy-minion.py
+++ b/minion/debdeploy-minion.py
@@ -39,11 +39,11 @@
     It is mostly used by other debdeploy Salt modules to determine whether 
packages
     were updated, installed or removed.
     '''
-    
+
     cmd = 'dpkg-query --showformat=\'${Status} ${Package} ' \
           '${Version} ${Architecture}\n\' -W'
     pkgs = {}
-    
+
     out = __salt__['cmd.run_stdout'](cmd, output_loglevel='debug')
     for line in out.splitlines():
         cols = line.split()
@@ -112,7 +112,7 @@
                         results[program] = 1
                 except OSError:
                     results[program] = 1
-            break    
+            break
 
 
         try:
@@ -123,7 +123,7 @@
                 break
 
         service = "undefined"
-    
+
         if os.path.exists('/bin/systemd'): # systemd
             cgroup = os.path.join("/proc", pid, "cgroup")
             if os.path.exists(cgroup):
@@ -132,13 +132,13 @@
                     if i.startswith("1:name"):
                         service = i.split("/")[-1].strip()
                 f.close()
-            
+
         elif os.path.exists('/sbin/initctl'): # upstart
             jobs = subprocess.check_output(["/sbin/initctl", "list"])
             for x in jobs.splitlines():
                 if x.endswith(str(pid)):
                     service = x.split()[0]
-                
+
         # no systemd or upstart job is present, let's check sysvinit
         if service == "undefined":
             # try a heuristic for sysvinit, in many cases the name of the 
daemon equals the init script name
@@ -150,7 +150,7 @@
 
             if os.path.exists(os.path.join('/etc/init.d/', program_basename)):
                 service = program_basename
-           
+
         logging.info("Restarting " + service + " for " + program)
         if __salt__['service.restart'](service):
             results[program] = 0
@@ -172,7 +172,7 @@
 
     except MinionError as exc:
         raise CommandExecutionError(exc)
-    
+
     if pkg_params is None or len(pkg_params) == 0:
         return {}
     if pkg_type == 'repository':
@@ -206,7 +206,7 @@
 
     pending_restarts_pre = set()
     pending_restarts_post = set()
-    
+
     installed_distro = grains['oscodename']
     if not versions.has_key(installed_distro):
         logging.info("Update doesn't apply to the installed distribution (" + 
installed_distro + ")")
@@ -222,7 +222,7 @@
             installed_binary_packages.append({pkg['Package'] : 
versions[installed_distro]})
         elif pkg.has_key('Package') and pkg['Package'] == source:
             installed_binary_packages.append({pkg['Package'] : 
versions[installed_distro]})
-            
+
     if len(installed_binary_packages) == 0:
         logging.info("No binary packages installed for source package " + 
source)
         return {}
@@ -230,7 +230,7 @@
     if update_type == "library":
         pending_restarts_pre = Checkrestart().get_programs_to_restart()
         logging.debug("Packages needing a restart prior to the update:" + 
str(pending_restarts_pre))
-    
+
     old = list_pkgs()
 
     logging.info("Refreshing apt package database")
@@ -251,22 +251,22 @@
     removals = []
     updated = []
     restarts = []
-    
+
     if update_type == "library":
         restarts = list(pending_restarts_post.difference(pending_restarts_pre))
-        
+
     for i in nk.difference(ok):
         additions.append[i]
     for i in ok.difference(nk):
         removals.append[i]
     intersect = ok.intersection(nk)
     modified = {x : (old[x], new[x]) for x in intersect if old[x] != new[x]}
-    
+
     logging.info("Newly installed packages:" + str(additions))
     logging.info("Removed packages: "  + str(removals))
     logging.info("Modified packages: " + str(modified))
     logging.info("Packages needing a restart: " + str(restarts))
-   
+
     r = {}
     r["additions"] = additions
     r["removals"] = removals
@@ -280,7 +280,7 @@
     jobfile = open("/var/lib/debdeploy/" + jobid + ".job", "w")
     pickle.dump(r, jobfile)
     jobfile.close()
-    
+
     return r
 
 
@@ -298,10 +298,10 @@
     aptstderr = ""
     aptstdout = ""
     aptreturn = 0
-    
+
     if len(r['updated'].keys()) > 0:
         pkgdowngrade = []
-        for i in r['updated'].keys():
+        for i in r['updated']:
             a = {}
             a[i] = r['updated'][i][0]
             pkgdowngrade.append(a)
@@ -309,7 +309,7 @@
         aptstderr += apt_call['stderr']
         aptstdout += apt_call['stdout']
         aptreturn += apt_call['retcode']
-        
+
     if len(r['removals']) > 0:
         install_pkgs(r['removals'])
         aptstderr += apt_call['stderr']
@@ -322,7 +322,7 @@
 
     if aptreturn > 100:
         aptreturn = 100
-        
+
     new = list_pkgs()
     ok = set(old.keys())
     nk = set(new.keys())
@@ -338,7 +338,7 @@
         removals.append[i]
     intersect = ok.intersection(nk)
     modified = {x : (old[x], new[x]) for x in intersect if old[x] != new[x]}
-    
+
     logging.info("Newly installed packages:" + str(additions))
     logging.info("Removed packages: "  + str(removals))
     logging.info(modified)
@@ -351,7 +351,7 @@
     r["aptlog"] = aptstdout
     r["apterrlog"] = aptstderr
     r["aptreturn"] = aptreturn
-    
+
     return r
 
 # Local variables:
diff --git a/minion/debdeploy_restart.py b/minion/debdeploy_restart.py
index 7180bde..77b1388 100644
--- a/minion/debdeploy_restart.py
+++ b/minion/debdeploy_restart.py
@@ -194,10 +194,10 @@
     def needsRestart(self, blacklist = None):
         for f in self.files:
             if isdeletedFile(f, blacklist):
-               return 1
-       for f in self.links:
-           if f == 0:
-               return 1
+                return 1
+        for f in self.links:
+            if f == 0:
+                return 1
         return 0
 
 class Checkrestart:
@@ -282,7 +282,7 @@
 
     def get_programs_to_restart(self):
         return set(self.programs.keys())
-    
+
     def lsoffilescheck(self, blacklist = None):
         processes = {}
 
@@ -324,16 +324,3 @@
         toRestart = filter(lambda process: process.needsRestart(blacklist),
                        processes.values())
         return toRestart
-
-
-
-
-
-
-
-
-
-
-
-
-
diff --git a/tests/test-jobdb b/tests/test-jobdb
index 45d9994..189bd4c 100755
--- a/tests/test-jobdb
+++ b/tests/test-jobdb
@@ -26,7 +26,7 @@
         f = open(yamlfile, "w")
         f.write(yaml1)
         f.close()
-        
+
     def testJobs(self):
 
         self.assertEqual(self.joblogdb.does_job_exist(self.yamlfile, 
"hostclass:testsystem"), False)
@@ -63,8 +63,3 @@
 # Local variables:
 # mode: python
 # End:
-
-
-
-
-
diff --git a/tests/test-pkgdb b/tests/test-pkgdb
index 10b044b..8bd686b 100755
--- a/tests/test-pkgdb
+++ b/tests/test-pkgdb
@@ -14,7 +14,7 @@
         if os.path.exists(name):
             os.remove(name)
         self.pkg = DebDeployPkgDB(name)
-        
+
     def testHosts(self):
         self.reinit_db("tests-host.sqlite")
         self.assertEqual(self.pkg.get_host_id("host1"), 0)
@@ -77,7 +77,7 @@
         self.assertEqual(self.pkg.get_outdated_packages("elinks", "0.11-4"), 
[])
         self.assertEqual(self.pkg.get_outdated_packages("elinks", "0.11-5"), 
["host1"])
         self.assertEqual(self.pkg.get_outdated_packages("elinks", "1:0.11-4"), 
["host1"])
-   
+
         self.pkg.add_host("host2")
         self.pkg.add_host("host3")
         inst = {}
@@ -103,18 +103,10 @@
         remove["elinks"] = "0.11-6"
         self.pkg.remove_installed_packages("host1", remove)
         self.assertEqual(self.pkg.get_installed_version("elinks", "host1"), 
None)
-      
+
 if __name__ == '__main__':
     unittest.main()
-         
+
 # Local variables:
 # mode: python
 # End:
-
-
-
-
-
-
-
-

-- 
To view, visit https://gerrit.wikimedia.org/r/228344
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I680695b7ad1939aa0e5cb90be81f6de949f79cbc
Gerrit-PatchSet: 1
Gerrit-Project: operations/debs/debdeploy
Gerrit-Branch: master
Gerrit-Owner: Muehlenhoff <[email protected]>
Gerrit-Reviewer: Muehlenhoff <[email protected]>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to