review cleanup

doc/source/development_saio.rst

@@ -531,7 +531,6 @@ Setting up scripts for running Swift
         #!/bin/bash
 
         swift-init all stop
-        sleep 5
         sudo umount /mnt/sdb1
         sudo mkfs.xfs -f -i size=1024 /dev/sdb1
         sudo mount /mnt/sdb1

doc/source/misc.rst

@@ -116,6 +116,13 @@ MemCacheD
     :members:
     :show-inheritance:
 
+Manager
+=========
+
+.. automodule:: swift.common.manager
+    :members:
+    :show-inheritance:
+
 Ratelimit
 =========
 

swift/common/manager.py

@@ -21,10 +21,11 @@ import resource
 import signal
 import sys
 import time
-from swift.common.utils import search_tree, remove_file, write_file
 import subprocess
 import re
 
+from swift.common.utils import search_tree, remove_file, write_file
+
 SWIFT_DIR = '/etc/swift'
 RUN_DIR = '/var/run/swift'
 
@@ -58,7 +59,6 @@ def setup_env():
                 "  Running as non-root?"
 
     os.environ['PYTHON_EGG_CACHE'] = '/tmp'
-    return
 
 
 def command(func):
@@ -88,7 +88,7 @@ def watch_server_pids(server_pids, interval=1, **kwargs):
     start = time.time()
     end = start + interval
     server_pids = dict(server_pids)  # make a copy
-    while interval:
+    while True:
         for server, pids in server_pids.items():
             for pid in pids:
                 try:
@@ -112,7 +112,6 @@ def watch_server_pids(server_pids, interval=1, **kwargs):
             break
         else:
             time.sleep(0.1)
-    return
 
 
 class UnknownCommandError(Exception):
@@ -227,7 +226,8 @@ class Manager():
         for server, pids in server_pids.items():
             if not killed_pids.issuperset(pids):
                 # some pids of this server were not killed
-                print 'Waited 15 seconds for %s to die; giving up' % (server)
+                print 'Waited %s seconds for %s to die; giving up' % (
+                    KILL_WAIT, server)
         return 1
 
     @command
@@ -314,7 +314,7 @@ class Server():
         if '-' not in server:
             server = '%s-server' % server
         self.server = server.lower()
-        self.type = '-'.join(server.split('-')[:-1])
+        self.type = server.rsplit('-', 1)[0]
         self.cmd = 'swift-%s' % server
         self.procs = []
 
@@ -360,11 +360,11 @@ class Server():
                     '.pid', 1)[0] + '.conf'
 
     def conf_files(self, **kwargs):
-        """Get ini files for this server
+        """Get conf files for this server
 
         :param: number, if supplied will only lookup the nth server
 
-        :returns: list of ini files
+        :returns: list of conf files
         """
         found_conf_files = search_tree(SWIFT_DIR, '%s-server*' % self.type,
                                       '.conf')
@@ -397,11 +397,9 @@ class Server():
         :returns: list of pid files
         """
         pid_files = search_tree(RUN_DIR, '%s*' % self.server, '.pid')
-        number = kwargs.get('number', 0)
+        if kwargs.get('number', 0):
-        if number:
             conf_files = self.conf_files(**kwargs)
-            # limt pid_files the one who translates to the indexed
+            # filter pid_files to match the index of numbered conf_file
-            # conf_file for this given number
             pid_files = [pid_file for pid_file in pid_files if
                          self.get_conf_file_name(pid_file) in conf_files]
         return pid_files
@@ -429,7 +427,7 @@ class Server():
                 os.kill(pid, sig)
             except OSError, e:
                 #print '%s sig err: %s' % (pid, e)
-                if e.errno == 3:
+                if e.errno == errno.ESRCH:
                     # pid does not exist
                     if kwargs.get('verbose'):
                         print "Removing stale pid file %s" % pid_file
@@ -533,6 +531,7 @@ class Server():
             output = proc.stdout.read()
             if output:
                 print output
+                proc.communicate()
             if proc.returncode:
                 status += 1
         return status
@@ -551,7 +550,7 @@ class Server():
 
     def launch(self, **kwargs):
         """
-        Collect ini files and attempt to spawn the processes for this server
+        Collect conf files and attempt to spawn the processes for this server
         """
         conf_files = self.conf_files(**kwargs)
         if not conf_files:
@@ -581,8 +580,6 @@ class Server():
         if self.server not in START_ONCE_SERVERS:
             kwargs['once'] = False
 
-        # TODO: check if self.cmd exists?
-
         pids = {}
         for conf_file in conf_files:
             if kwargs.get('once'):
@@ -594,7 +591,7 @@ class Server():
                 pid = self.spawn(conf_file, **kwargs)
             except OSError, e:
                 if e.errno == errno.ENOENT:
-                    # cmd does not exist
+                    # TODO: should I check if self.cmd exists earlier?
                     print "%s does not exist" % self.cmd
                     break
             pids[pid] = conf_file

swift/common/utils.py

@@ -77,9 +77,8 @@ TRUE_VALUES = set(('true', '1', 'yes', 'True', 'Yes', 'on', 'On'))
 
 def validate_configuration():
     if HASH_PATH_SUFFIX == '':
-        print "Error: [swift-hash]: swift_hash_path_suffix missing from " \
+        sys.exit("Error: [swift-hash]: swift_hash_path_suffix missing "
-                "/etc/swift/swift.conf"
+                 "from /etc/swift/swift.conf")
-        sys.exit(1)
 
 
 def load_libc_function(func_name):
@@ -822,13 +821,14 @@ def write_file(path, contents):
     :param contents: data to write to file, will be converted to string
 
     """
-    dir, name = os.path.split(path)
+    dirname, name = os.path.split(path)
-    if not os.path.exists(dir):
+    if not os.path.exists(dirname):
         try:
-            os.makedirs(dir)
+            os.makedirs(dirname)
         except OSError, err:
             if err.errno == errno.EACCES:
-                sys.exit('Unable to create %s.  Running as non-root?' % dir)
+                sys.exit('Unable to create %s.  Running as '
+                         'non-root?' % dirname)
     with open(path, 'w') as f:
         f.write('%s' % contents)
 

swift/common/wsgi.py

@@ -119,9 +119,10 @@ def run_wsgi(conf_file, app_section, *args, **kwargs):
         logger = get_logger(conf, log_name,
             log_to_console=kwargs.pop('verbose', False), log_route='wsgi')
 
+    # TODO: should we wait to close stdio until after we've created the socket
+    # and initialized the app?
     # redirect errors to logger and close stdio
     capture_stdio(logger)
-
     # bind to address and port
     sock = get_socket(conf, default_port=kwargs.get('default_port', 8080))
     # remaining tasks should not require elevated privileges

test/unit/common/test_manager.py

@@ -204,7 +204,7 @@ class TestManagerModule(unittest.TestCase):
             # basic test, server dies
             gen = manager.watch_server_pids(server_pids)
             expected = [(server, 1)]
-            self.assertEquals([x for x in gen], [(server, 1)])
+            self.assertEquals([x for x in gen], expected)
             # start long running server and short interval
             server = MockServer([1], zombie=15)
             server_pids = {
@@ -886,7 +886,10 @@ class TestServer(unittest.TestCase):
                 self.finished = False
                 self.returncode = None
                 if fail_to_start:
+                    self._returncode = 1
                     self.run = self.fail
+                else:
+                    self._returncode = 0
 
             def __enter__(self):
                 self.start()
@@ -908,9 +911,11 @@ class TestServer(unittest.TestCase):
                 print >>self._stdout, 'mock process started'
                 sleep(self.delay)  # perform setup processing
                 print >>self._stdout, 'mock process failed to start'
-                self.returncode = 1
                 self.close_stdout()
 
+            def communicate(self):
+                self.returncode = self._returncode
+
             def run(self):
                 print >>self._stdout, 'mock process started'
                 sleep(self.delay)  # perform setup processing