merged trunk and fixed merge errors

bin/nova-api-new

@@ -18,17 +18,17 @@
 #    See the License for the specific language governing permissions and
 #    limitations under the License.
 """
-  Daemon for the Rackspace API endpoint.
+Nova API daemon.
 """
 
+from nova import api
 from nova import flags
 from nova import utils
 from nova import wsgi
-from nova.endpoint import newapi
 
 FLAGS = flags.FLAGS
-flags.DEFINE_integer('cc_port', 8773, 'cloud controller port')
+flags.DEFINE_integer('api_port', 8773, 'API port')
 
 if __name__ == '__main__':
     utils.default_flagfile()
-    wsgi.run_server(newapi.APIVersionRouter(), FLAGS.cc_port)
+    wsgi.run_server(api.API(), FLAGS.api_port)

bin/nova-compute

@@ -29,4 +29,4 @@ if __name__ == '__main__':
     twistd.serve(__file__)
 
 if __name__ == '__builtin__':
-    application = service.ComputeService.create()
+    application = service.ComputeService.create() # pylint: disable-msg=C0103

bin/nova-dhcpbridge

@@ -41,7 +41,7 @@ from nova.auth import manager # for auth flags
 FLAGS = flags.FLAGS
 
 
-def add_lease(_mac, ip, _hostname, _interface):
+def add_lease(_mac, ip_address, _hostname, _interface):
     """Set the IP that was assigned by the DHCP server."""
     if FLAGS.fake_rabbit:
         logging.debug("leasing ip")
@@ -49,27 +49,27 @@ def add_lease(_mac, ip, _hostname, _interface):
         print models.FixedIp.count()
         print models.Network.count()
         print FLAGS.sql_connection
-        service.VlanNetworkService().lease_fixed_ip(ip)
+        service.VlanNetworkService().lease_fixed_ip(ip_address)
     else:
         rpc.cast("%s.%s" % (FLAGS.network_topic, FLAGS.node_name),
                  {"method": "lease_fixed_ip",
-                  "args": {"address": ip}})
+                  "args": {"address": ip_address}})
 
 
-def old_lease(_mac, _ip, _hostname, _interface):
+def old_lease(_mac, _ip_address, _hostname, _interface):
     """Do nothing, just an old lease update."""
     logging.debug("Adopted old lease or got a change of mac/hostname")
 
 
-def del_lease(_mac, ip, _hostname, _interface):
+def del_lease(_mac, ip_address, _hostname, _interface):
     """Called when a lease expires."""
     if FLAGS.fake_rabbit:
         logging.debug("releasing ip")
-        service.VlanNetworkService().release_fixed_ip(ip)
+        service.VlanNetworkService().release_fixed_ip(ip_address)
     else:
         rpc.cast("%s.%s" % (FLAGS.network_topic, FLAGS.node_name),
                  {"method": "release_fixed_ip",
-                  "args": {"address": ip}})
+                  "args": {"fixed_ip": ip_address}})
 
 
 def init_leases(interface):

bin/nova-import-canonical-imagestore

@@ -35,12 +35,12 @@ from nova.objectstore import image
 
 FLAGS = flags.FLAGS
 
-api_url = 'https://imagestore.canonical.com/api/dashboard'
+API_URL = 'https://imagestore.canonical.com/api/dashboard'
 
 
 def get_images():
     """Get a list of the images from the imagestore URL."""
-    images = json.load(urllib2.urlopen(api_url))['images']
+    images = json.load(urllib2.urlopen(API_URL))['images']
     images = [img for img in images if img['title'].find('amd64') > -1]
     return images
 
@@ -56,21 +56,21 @@ def download(img):
     for f in img['files']:
         if f['kind'] == 'kernel':
             dest = os.path.join(tempdir, 'kernel')
-            subprocess.call(['curl', f['url'], '-o', dest])
+            subprocess.call(['curl', '--fail', f['url'], '-o', dest])
             kernel_id = image.Image.add(dest,
                 description='kernel/' + img['title'], kernel=True)
 
     for f in img['files']:
         if f['kind'] == 'ramdisk':
             dest = os.path.join(tempdir, 'ramdisk')
-            subprocess.call(['curl', f['url'], '-o', dest])
+            subprocess.call(['curl', '--fail', f['url'], '-o', dest])
             ramdisk_id = image.Image.add(dest,
                 description='ramdisk/' + img['title'], ramdisk=True)
 
     for f in img['files']:
         if f['kind'] == 'image':
             dest = os.path.join(tempdir, 'image')
-            subprocess.call(['curl', f['url'], '-o', dest])
+            subprocess.call(['curl', '--fail', f['url'], '-o', dest])
             ramdisk_id = image.Image.add(dest,
                 description=img['title'], kernel=kernel_id, ramdisk=ramdisk_id)
 

bin/nova-instancemonitor

@@ -35,9 +35,10 @@ if __name__ == '__main__':
 
 if __name__ == '__builtin__':
     logging.warn('Starting instance monitor')
-    m = monitor.InstanceMonitor()
+    # pylint: disable-msg=C0103
+    monitor = monitor.InstanceMonitor()
 
     # This is the parent service that twistd will be looking for when it
     # parses this file, return it so that we can get it into globals below
     application = service.Application('nova-instancemonitor')
-    m.setServiceParent(application)
+    monitor.setServiceParent(application)

bin/nova-manage

@@ -56,7 +56,8 @@ class VpnCommands(object):
             vpn = self._vpn_for(project.id)
             if vpn:
                 command = "ping -c1 -w1 %s > /dev/null; echo $?"
-                out, _err = utils.execute(command % vpn['private_dns_name'])
+                out, _err = utils.execute(  command % vpn['private_dns_name'],
+                                            check_exit_code=False)
                 if out.strip() == '0':
                     net = 'up'
                 else:
@@ -211,7 +212,7 @@ class ProjectCommands(object):
             f.write(zip_file)
 
 
-categories = [
+CATEGORIES = [
     ('user', UserCommands),
     ('project', ProjectCommands),
     ('role', RoleCommands),
@@ -258,11 +259,11 @@ def main():
     if len(argv) < 1:
         print script_name + " category action [<args>]"
         print "Available categories:"
-        for k, _ in categories:
+        for k, _ in CATEGORIES:
             print "\t%s" % k
         sys.exit(2)
     category = argv.pop(0)
-    matches = lazy_match(category, categories)
+    matches = lazy_match(category, CATEGORIES)
     # instantiate the command group object
     category, fn = matches[0]
     command_object = fn()

bin/nova-network

@@ -33,4 +33,5 @@ if __name__ == '__main__':
     twistd.serve(__file__)
 
 if __name__ == '__builtin__':
+    # pylint: disable-msg=C0103
     application = service.type_to_class(FLAGS.network_type).create()

bin/nova-objectstore

@@ -35,4 +35,4 @@ if __name__ == '__main__':
 
 if __name__ == '__builtin__':
     utils.default_flagfile()
-    application = handler.get_application()
+    application = handler.get_application() # pylint: disable-msg=C0103

bin/nova-volume

@@ -29,4 +29,4 @@ if __name__ == '__main__':
     twistd.serve(__file__)
 
 if __name__ == '__builtin__':
-    application = service.VolumeService.create()
+    application = service.VolumeService.create() # pylint: disable-msg=C0103

nova/auth/fakeldap.py

@@ -35,15 +35,18 @@ MOD_ADD = 0
 MOD_DELETE = 1
 
 
-class NO_SUCH_OBJECT(Exception):
+class NO_SUCH_OBJECT(Exception): # pylint: disable-msg=C0103
+    """Duplicate exception class from real LDAP module."""
     pass
 
 
-class OBJECT_CLASS_VIOLATION(Exception):
+class OBJECT_CLASS_VIOLATION(Exception): # pylint: disable-msg=C0103
+    """Duplicate exception class from real LDAP module."""
     pass
 
 
-def initialize(uri):
+def initialize(_uri):
+    """Opens a fake connection with an LDAP server."""
     return FakeLDAP()
 
 
@@ -68,7 +71,7 @@ def _match_query(query, attrs):
         # cut off the ! and the nested parentheses
         return not _match_query(query[2:-1], attrs)
 
-    (k, sep, v) = inner.partition('=')
+    (k, _sep, v) = inner.partition('=')
     return _match(k, v, attrs)
 
 
@@ -89,16 +92,16 @@ def _paren_groups(source):
     return result
 
 
-def _match(k, v, attrs):
+def _match(key, value, attrs):
     """Match a given key and value against an attribute list."""
-    if k not in attrs:
+    if key not in attrs:
         return False
-    if k != "objectclass":
+    if key != "objectclass":
-        return v in attrs[k]
+        return value in attrs[key]
     # it is an objectclass check, so check subclasses
-    values = _subs(v)
+    values = _subs(value)
-    for value in values:
+    for v in values:
-        if value in attrs[k]:
+        if v in attrs[key]:
             return True
     return False
 
@@ -145,6 +148,7 @@ def _to_json(unencoded):
 class FakeLDAP(object):
     #TODO(vish): refactor this class to use a wrapper instead of accessing
     #            redis directly
+    """Fake LDAP connection."""
 
     def simple_bind_s(self, dn, password):
         """This method is ignored, but provided for compatibility."""
@@ -207,6 +211,7 @@ class FakeLDAP(object):
             # get the attributes from redis
             attrs = redis.hgetall(key)
             # turn the values from redis into lists
+            # pylint: disable-msg=E1103
             attrs = dict([(k, _from_json(v))
                           for k, v in attrs.iteritems()])
             # filter the objects by query
@@ -215,12 +220,12 @@ class FakeLDAP(object):
                 attrs = dict([(k, v) for k, v in attrs.iteritems()
                               if not fields or k in fields])
                 objects.append((key[len(self.__redis_prefix):], attrs))
+            # pylint: enable-msg=E1103
         if objects == []:
             raise NO_SUCH_OBJECT()
         return objects
 
     @property
-    def __redis_prefix(self):
+    def __redis_prefix(self): # pylint: disable-msg=R0201
+        """Get the prefix to use for all redis keys."""
         return 'ldap:'
-
-

nova/auth/ldapdriver.py

@@ -63,14 +63,18 @@ flags.DEFINE_string('ldap_developer',
 #             to define a set interface for AuthDrivers. I'm delaying
 #             creating this now because I'm expecting an auth refactor
 #             in which we may want to change the interface a bit more.
+
+
 class LdapDriver(object):
     """Ldap Auth driver
 
     Defines enter and exit and therefore supports the with/as syntax.
     """
+
     def __init__(self):
         """Imports the LDAP module"""
         self.ldap = __import__('ldap')
+        self.conn = None
 
     def __enter__(self):
         """Creates the connection to LDAP"""
@@ -78,7 +82,7 @@ class LdapDriver(object):
         self.conn.simple_bind_s(FLAGS.ldap_user_dn, FLAGS.ldap_password)
         return self
 
-    def __exit__(self, type, value, traceback):
+    def __exit__(self, exc_type, exc_value, traceback):
         """Destroys the connection to LDAP"""
         self.conn.unbind_s()
         return False
@@ -123,11 +127,11 @@ class LdapDriver(object):
 
     def get_projects(self, uid=None):
         """Retrieve list of projects"""
-        filter = '(objectclass=novaProject)'
+        pattern = '(objectclass=novaProject)'
         if uid:
-            filter = "(&%s(member=%s))" % (filter, self.__uid_to_dn(uid))
+            pattern = "(&%s(member=%s))" % (pattern, self.__uid_to_dn(uid))
         attrs = self.__find_objects(FLAGS.ldap_project_subtree,
-                                  filter)
+                                    pattern)
         return [self.__to_project(attr) for attr in attrs]
 
     def create_user(self, name, access_key, secret_key, is_admin):
@@ -194,8 +198,7 @@ class LdapDriver(object):
             ('cn', [name]),
             ('description', [description]),
             ('projectManager', [manager_dn]),
-            ('member', members)
+            ('member', members)]
-        ]
         self.conn.add_s('cn=%s,%s' % (name, FLAGS.ldap_project_subtree), attr)
         return self.__to_project(dict(attr))
 
@@ -287,7 +290,6 @@ class LdapDriver(object):
 
     def __key_pair_exists(self, uid, key_name):
         """Check if key pair exists"""
-        return self.get_user(uid) != None
         return self.get_key_pair(uid, key_name) != None
 
     def __project_exists(self, project_id):
@@ -310,7 +312,7 @@ class LdapDriver(object):
         except self.ldap.NO_SUCH_OBJECT:
             return []
         # just return the DNs
-        return [dn for dn, attributes in res]
+        return [dn for dn, _attributes in res]
 
     def __find_objects(self, dn, query=None, scope=None):
         """Find objects by query"""
@@ -346,7 +348,8 @@ class LdapDriver(object):
             for key in keys:
                 self.delete_key_pair(uid, key['name'])
 
-    def __role_to_dn(self, role, project_id=None):
+    @staticmethod
+    def __role_to_dn(role, project_id=None):
         """Convert role to corresponding dn"""
         if project_id == None:
             return FLAGS.__getitem__("ldap_%s" % role).value
@@ -375,8 +378,7 @@ class LdapDriver(object):
             ('objectclass', ['groupOfNames']),
             ('cn', [name]),
             ('description', [description]),
-            ('member', members)
+            ('member', members)]
-        ]
         self.conn.add_s(group_dn, attr)
 
     def __is_in_group(self, uid, group_dn):
@@ -402,9 +404,7 @@ class LdapDriver(object):
         if self.__is_in_group(uid, group_dn):
             raise exception.Duplicate("User %s is already a member of "
                                       "the group %s" % (uid, group_dn))
-        attr = [
+        attr = [(self.ldap.MOD_ADD, 'member', self.__uid_to_dn(uid))]
-            (self.ldap.MOD_ADD, 'member', self.__uid_to_dn(uid))
-        ]
         self.conn.modify_s(group_dn, attr)
 
     def __remove_from_group(self, uid, group_dn):
@@ -432,7 +432,7 @@ class LdapDriver(object):
             self.conn.modify_s(group_dn, attr)
         except self.ldap.OBJECT_CLASS_VIOLATION:
             logging.debug("Attempted to remove the last member of a group. "
-                          "Deleting the group at %s instead." % group_dn )
+                          "Deleting the group at %s instead.", group_dn)
             self.__delete_group(group_dn)
 
     def __remove_from_all(self, uid):
@@ -440,7 +440,6 @@ class LdapDriver(object):
         if not self.__user_exists(uid):
             raise exception.NotFound("User %s can't be removed from all "
                     "because the user doesn't exist" % (uid,))
-        dn = self.__uid_to_dn(uid)
         role_dns = self.__find_group_dns_with_member(
                 FLAGS.role_project_subtree, uid)
         for role_dn in role_dns:
@@ -448,7 +447,7 @@ class LdapDriver(object):
         project_dns = self.__find_group_dns_with_member(
                 FLAGS.ldap_project_subtree, uid)
         for project_dn in project_dns:
-            self.__safe_remove_from_group(uid, role_dn)
+            self.__safe_remove_from_group(uid, project_dn)
 
     def __delete_group(self, group_dn):
         """Delete Group"""
@@ -461,7 +460,8 @@ class LdapDriver(object):
         for role_dn in self.__find_role_dns(project_dn):
             self.__delete_group(role_dn)
 
-    def __to_user(self, attr):
+    @staticmethod
+    def __to_user(attr):
         """Convert ldap attributes to User object"""
         if attr == None:
             return None
@@ -470,10 +470,10 @@ class LdapDriver(object):
             'name': attr['cn'][0],
             'access': attr['accessKey'][0],
             'secret': attr['secretKey'][0],
-            'admin': (attr['isAdmin'][0] == 'TRUE')
+            'admin': (attr['isAdmin'][0] == 'TRUE')}
-        }
 
-    def __to_key_pair(self, owner, attr):
+    @staticmethod
+    def __to_key_pair(owner, attr):
         """Convert ldap attributes to KeyPair object"""
         if attr == None:
             return None
@@ -482,8 +482,7 @@ class LdapDriver(object):
             'name': attr['cn'][0],
             'owner_id': owner,
             'public_key': attr['sshPublicKey'][0],
-            'fingerprint': attr['keyFingerprint'][0],
+            'fingerprint': attr['keyFingerprint'][0]}
-        }
 
     def __to_project(self, attr):
         """Convert ldap attributes to Project object"""
@@ -495,21 +494,22 @@ class LdapDriver(object):
             'name': attr['cn'][0],
             'project_manager_id': self.__dn_to_uid(attr['projectManager'][0]),
             'description': attr.get('description', [None])[0],
-            'member_ids': [self.__dn_to_uid(x) for x in member_dns]
+            'member_ids': [self.__dn_to_uid(x) for x in member_dns]}
-        }
 
-    def __dn_to_uid(self, dn):
+    @staticmethod
+    def __dn_to_uid(dn):
         """Convert user dn to uid"""
         return dn.split(',')[0].split('=')[1]
 
-    def __uid_to_dn(self, dn):
+    @staticmethod
+    def __uid_to_dn(dn):
         """Convert uid to dn"""
         return 'uid=%s,%s' % (dn, FLAGS.ldap_user_subtree)
 
 
 class FakeLdapDriver(LdapDriver):
     """Fake Ldap Auth driver"""
-    def __init__(self):
+
+    def __init__(self): # pylint: disable-msg=W0231
         __import__('nova.auth.fakeldap')
         self.ldap = sys.modules['nova.auth.fakeldap']
-

nova/auth/manager.py

@@ -23,7 +23,7 @@ Nova authentication management
 import logging
 import os
 import shutil
-import string
+import string # pylint: disable-msg=W0402
 import tempfile
 import uuid
 import zipfile
@@ -32,7 +32,6 @@ from nova import crypto
 from nova import db
 from nova import exception
 from nova import flags
-from nova import models
 from nova import utils
 from nova.auth import signer
 
@@ -195,12 +194,12 @@ class Project(AuthBase):
 
     @property
     def vpn_ip(self):
-        ip, port = AuthManager().get_project_vpn_data(self)
+        ip, _port = AuthManager().get_project_vpn_data(self)
         return ip
 
     @property
     def vpn_port(self):
-        ip, port = AuthManager().get_project_vpn_data(self)
+        _ip, port = AuthManager().get_project_vpn_data(self)
         return port
 
     def has_manager(self, user):
@@ -222,10 +221,8 @@ class Project(AuthBase):
         return AuthManager().get_credentials(user, self)
 
     def __repr__(self):
-        return "Project('%s', '%s', '%s', '%s', %s)" % (self.id,
+        return "Project('%s', '%s', '%s', '%s', %s)" % \
-                                                        self.name,
+            (self.id, self.name, self.project_manager_id, self.description,
-                                                        self.project_manager_id,
-                                                        self.description,
              self.member_ids)
 
 
@@ -298,7 +295,7 @@ class AuthManager(object):
         @return: User and project that the request represents.
         """
         # TODO(vish): check for valid timestamp
-        (access_key, sep, project_id) = access.partition(':')
+        (access_key, _sep, project_id) = access.partition(':')
 
         logging.info('Looking up user: %r', access_key)
         user = self.get_user_from_access_key(access_key)
@@ -321,7 +318,8 @@ class AuthManager(object):
             raise exception.NotFound('User %s is not a member of project %s' %
                                      (user.id, project.id))
         if check_type == 's3':
-            expected_signature = signer.Signer(user.secret.encode()).s3_authorization(headers, verb, path)
+            sign = signer.Signer(user.secret.encode())
+            expected_signature = sign.s3_authorization(headers, verb, path)
             logging.debug('user.secret: %s', user.secret)
             logging.debug('expected_signature: %s', expected_signature)
             logging.debug('signature: %s', signature)
@@ -466,7 +464,8 @@ class AuthManager(object):
         with self.driver() as drv:
             drv.remove_role(User.safe_id(user), role, Project.safe_id(project))
 
-    def get_roles(self, project_roles=True):
+    @staticmethod
+    def get_roles(project_roles=True):
         """Get list of allowed roles"""
         if project_roles:
             return list(set(FLAGS.allowed_roles) - set(FLAGS.global_roles))
@@ -553,7 +552,8 @@ class AuthManager(object):
             return drv.remove_from_project(User.safe_id(user),
                                             Project.safe_id(project))
 
-    def get_project_vpn_data(self, project, context=None):
+    @staticmethod
+    def get_project_vpn_data(project, context=None):
         """Gets vpn ip and port for project
 
         @type project: Project or project_id
@@ -563,11 +563,9 @@ class AuthManager(object):
         @return: A tuple containing (ip, port) or None, None if vpn has
         not been allocated for user.
         """
-        # FIXME(vish): this shouldn't be messing with the datamodel directly
-        if not isinstance(project, Project):
-            project = self.get_project(project)
 
-        network_ref = db.project_get_network(context, project.id)
+        network_ref = db.project_get_network(context,
+                                             Project.safe_id(project))
 
         if not network_ref['vpn_public_port']:
             raise exception.NotFound('project network data has not been set')
@@ -577,9 +575,8 @@ class AuthManager(object):
     def delete_project(self, project, context=None):
         """Deletes a project"""
         # FIXME(ja): EVIL HACK
-        if not isinstance(project, Project):
+        network_ref = db.project_get_network(context,
-            project = self.get_project(project)
+                                             Project.safe_id(project))
-        network_ref = db.project_get_network(context, project.id)
         try:
             db.network_destroy(context, network_ref['id'])
         except:
@@ -632,8 +629,10 @@ class AuthManager(object):
         @rtype: User
         @return: The new user.
         """
-        if access == None: access = str(uuid.uuid4())
+        if access == None:
-        if secret == None: secret = str(uuid.uuid4())
+            access = str(uuid.uuid4())
+        if secret == None:
+            secret = str(uuid.uuid4())
         with self.driver() as drv:
             user_dict = drv.create_user(name, access, secret, admin)
             if user_dict:
@@ -736,10 +735,10 @@ class AuthManager(object):
         zippy.writestr(FLAGS.ca_file, crypto.fetch_ca(user.id))
         zippy.close()
         with open(zf, 'rb') as f:
-            buffer = f.read()
+            read_buffer = f.read()
 
         shutil.rmtree(tmpdir)
-        return buffer
+        return read_buffer
 
     def get_environment_rc(self, user, project=None):
         """Get credential zip for user in project"""
@@ -750,7 +749,8 @@ class AuthManager(object):
         pid = Project.safe_id(project)
         return self.__generate_rc(user.access, user.secret, pid)
 
-    def __generate_rc(self, access, secret, pid):
+    @staticmethod
+    def __generate_rc(access, secret, pid):
         """Generate rc file for user"""
         rc = open(FLAGS.credentials_template).read()
         rc = rc % {'access': access,
@@ -760,8 +760,7 @@ class AuthManager(object):
                    's3': 'http://%s:%s' % (FLAGS.s3_host, FLAGS.s3_port),
                    'nova': FLAGS.ca_file,
                    'cert': FLAGS.credential_cert_file,
-                    'key': FLAGS.credential_key_file,
+                   'key': FLAGS.credential_key_file}
-            }
         return rc
 
     def _generate_x509_cert(self, uid, pid):
@@ -772,6 +771,7 @@ class AuthManager(object):
         signed_cert = crypto.sign_csr(csr, pid)
         return (private_key, signed_cert)
 
-    def __cert_subject(self, uid):
+    @staticmethod
+    def __cert_subject(uid):
         """Helper to generate cert subject"""
         return FLAGS.credential_cert_subject % (uid, utils.isotime())

nova/auth/rbac.py

@@ -16,40 +16,54 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+"""Role-based access control decorators to use fpr wrapping other
+methods with."""
+
 from nova import exception
-from nova.auth import manager
 
 
 def allow(*roles):
-    def wrap(f):
+    """Allow the given roles access the wrapped function."""
-        def wrapped_f(self, context, *args, **kwargs):
+
+    def wrap(func): # pylint: disable-msg=C0111
+
+        def wrapped_func(self, context, *args,
+                         **kwargs): # pylint: disable-msg=C0111
             if context.user.is_superuser():
-                return f(self, context, *args, **kwargs)
+                return func(self, context, *args, **kwargs)
             for role in roles:
                 if __matches_role(context, role):
-                    return f(self, context, *args, **kwargs)
+                    return func(self, context, *args, **kwargs)
             raise exception.NotAuthorized()
-        return wrapped_f
+
+        return wrapped_func
+
     return wrap
 
 
 def deny(*roles):
-    def wrap(f):
+    """Deny the given roles access the wrapped function."""
-        def wrapped_f(self, context, *args, **kwargs):
+
+    def wrap(func): # pylint: disable-msg=C0111
+
+        def wrapped_func(self, context, *args,
+                         **kwargs): # pylint: disable-msg=C0111
             if context.user.is_superuser():
-                return f(self, context, *args, **kwargs)
+                return func(self, context, *args, **kwargs)
             for role in roles:
                 if __matches_role(context, role):
                     raise exception.NotAuthorized()
-            return f(self, context, *args, **kwargs)
+            return func(self, context, *args, **kwargs)
-        return wrapped_f
+
+        return wrapped_func
+
     return wrap
 
 
 def __matches_role(context, role):
+    """Check if a role is allowed."""
     if role == 'all':
         return True
     if role == 'none':
         return False
     return context.project.has_role(context.user.id, role)
-

nova/auth/signer.py

@@ -58,7 +58,7 @@ from nova.exception import Error
 
 
 class Signer(object):
-    """ hacked up code from boto/connection.py """
+    """Hacked up code from boto/connection.py"""
 
     def __init__(self, secret_key):
         self.hmac = hmac.new(secret_key, digestmod=hashlib.sha1)
@@ -66,22 +66,27 @@ class Signer(object):
             self.hmac_256 = hmac.new(secret_key, digestmod=hashlib.sha256)
 
     def s3_authorization(self, headers, verb, path):
+        """Generate S3 authorization string."""
         c_string = boto.utils.canonical_string(verb, path, headers)
-        hmac = self.hmac.copy()
+        hmac_copy = self.hmac.copy()
-        hmac.update(c_string)
+        hmac_copy.update(c_string)
-        b64_hmac = base64.encodestring(hmac.digest()).strip()
+        b64_hmac = base64.encodestring(hmac_copy.digest()).strip()
         return b64_hmac
 
     def generate(self, params, verb, server_string, path):
+        """Generate auth string according to what SignatureVersion is given."""
         if params['SignatureVersion'] == '0':
             return self._calc_signature_0(params)
         if params['SignatureVersion'] == '1':
             return self._calc_signature_1(params)
         if params['SignatureVersion'] == '2':
             return self._calc_signature_2(params, verb, server_string, path)
-        raise Error('Unknown Signature Version: %s' % self.SignatureVersion)
+        raise Error('Unknown Signature Version: %s' %
+                    params['SignatureVersion'])
 
-    def _get_utf8_value(self, value):
+    @staticmethod
+    def _get_utf8_value(value):
+        """Get the UTF8-encoded version of a value."""
         if not isinstance(value, str) and not isinstance(value, unicode):
             value = str(value)
         if isinstance(value, unicode):
@@ -90,6 +95,7 @@ class Signer(object):
             return value
 
     def _calc_signature_0(self, params):
+        """Generate AWS signature version 0 string."""
         s = params['Action'] + params['Timestamp']
         self.hmac.update(s)
         keys = params.keys()
@@ -101,6 +107,7 @@ class Signer(object):
         return base64.b64encode(self.hmac.digest())
 
     def _calc_signature_1(self, params):
+        """Generate AWS signature version 1 string."""
         keys = params.keys()
         keys.sort(cmp=lambda x, y: cmp(x.lower(), y.lower()))
         pairs = []
@@ -112,30 +119,34 @@ class Signer(object):
         return base64.b64encode(self.hmac.digest())
 
     def _calc_signature_2(self, params, verb, server_string, path):
+        """Generate AWS signature version 2 string."""
         logging.debug('using _calc_signature_2')
         string_to_sign = '%s\n%s\n%s\n' % (verb, server_string, path)
         if self.hmac_256:
-            hmac = self.hmac_256
+            current_hmac = self.hmac_256
             params['SignatureMethod'] = 'HmacSHA256'
         else:
-            hmac = self.hmac
+            current_hmac = self.hmac
             params['SignatureMethod'] = 'HmacSHA1'
         keys = params.keys()
         keys.sort()
         pairs = []
         for key in keys:
             val = self._get_utf8_value(params[key])
-            pairs.append(urllib.quote(key, safe='') + '=' + urllib.quote(val, safe='-_~'))
+            val = urllib.quote(val, safe='-_~')
+            pairs.append(urllib.quote(key, safe='') + '=' + val)
         qs = '&'.join(pairs)
-        logging.debug('query string: %s' % qs)
+        logging.debug('query string: %s', qs)
         string_to_sign += qs
-        logging.debug('string_to_sign: %s' % string_to_sign)
+        logging.debug('string_to_sign: %s', string_to_sign)
-        hmac.update(string_to_sign)
+        current_hmac.update(string_to_sign)
-        b64 = base64.b64encode(hmac.digest())
+        b64 = base64.b64encode(current_hmac.digest())
-        logging.debug('len(b64)=%d' % len(b64))
+        logging.debug('len(b64)=%d', len(b64))
-        logging.debug('base64 encoded digest: %s' % b64)
+        logging.debug('base64 encoded digest: %s', b64)
         return b64
 
 
 if __name__ == '__main__':
-    print Signer('foo').generate({"SignatureMethod": 'HmacSHA256', 'SignatureVersion': '2'}, "get", "server", "/foo")
+    print Signer('foo').generate({'SignatureMethod': 'HmacSHA256',
+                                  'SignatureVersion': '2'},
+                                 'get', 'server', '/foo')

nova/endpoint/aws/__init__.py

@@ -1,22 +0,0 @@
-import routes
-import webob.dec
-
-from nova import wsgi
-
-# TODO(gundlach): temp
-class API(wsgi.Router):
-    """WSGI entry point for all AWS API requests."""
-
-    def __init__(self):
-        mapper = routes.Mapper()
-
-        mapper.connect(None, "{all:.*}", controller=self.dummy)
-
-        super(API, self).__init__(mapper)
-
-    @webob.dec.wsgify
-    def dummy(self, req):
-        #TODO(gundlach)
-        msg = "dummy response -- please hook up __init__() to cloud.py instead"
-        return repr({ 'dummy': msg, 
-                 'kwargs': repr(req.environ['wsgiorg.routing_args'][1]) })

nova/endpoint/newapi.py

@@ -1,51 +0,0 @@
-# vim: tabstop=4 shiftwidth=4 softtabstop=4
-
-# Copyright 2010 United States Government as represented by the
-# Administrator of the National Aeronautics and Space Administration.
-# All Rights Reserved.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License"); you may
-#    not use this file except in compliance with the License. You may obtain
-#    a copy of the License at
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-"""
-:mod:`nova.endpoint` -- Main NOVA Api endpoints
-=====================================================
-
-.. automodule:: nova.endpoint
-   :platform: Unix
-   :synopsis: REST APIs for all nova functions
-.. moduleauthor:: Jesse Andrews <jesse@ansolabs.com>
-.. moduleauthor:: Devin Carlen <devin.carlen@gmail.com>
-.. moduleauthor:: Vishvananda Ishaya <vishvananda@yahoo.com>
-.. moduleauthor:: Joshua McKenty <joshua@cognition.ca>
-.. moduleauthor:: Manish Singh <yosh@gimp.org>
-.. moduleauthor:: Andy Smith <andy@anarkystic.com>
-"""
-
-from nova import wsgi
-import routes
-from nova.endpoint import rackspace
-from nova.endpoint import aws
-
-class APIVersionRouter(wsgi.Router):
-    """Routes top-level requests to the appropriate API."""
-
-    def __init__(self):
-        mapper = routes.Mapper()
-
-        rsapi = rackspace.API()
-        mapper.connect(None, "/v1.0/{path_info:.*}", controller=rsapi)
-
-        mapper.connect(None, "/ec2/{path_info:.*}", controller=aws.API())
-
-        super(APIVersionRouter, self).__init__(mapper)
-

nova/endpoint/rackspace/__init__.py

@@ -1,83 +0,0 @@
-# vim: tabstop=4 shiftwidth=4 softtabstop=4
-
-# Copyright 2010 United States Government as represented by the
-# Administrator of the National Aeronautics and Space Administration.
-# All Rights Reserved.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License"); you may
-#    not use this file except in compliance with the License. You may obtain
-#    a copy of the License at
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-"""
-Rackspace API Endpoint
-"""
-
-import json
-import time
-
-import webob.dec
-import webob.exc
-import routes
-
-from nova import flags
-from nova import wsgi
-from nova.auth import manager
-from nova.endpoint.rackspace import controllers
-
-
-class API(wsgi.Middleware):
-    """WSGI entry point for all Rackspace API requests."""
-
-    def __init__(self):
-        app = AuthMiddleware(APIRouter())
-        super(API, self).__init__(app)
-
-
-class AuthMiddleware(wsgi.Middleware):
-    """Authorize the rackspace API request or return an HTTP Forbidden."""
-
-    #TODO(gundlach): isn't this the old Nova API's auth?  Should it be replaced
-    #with correct RS API auth?
-
-    @webob.dec.wsgify
-    def __call__(self, req):
-        context = {}
-        if "HTTP_X_AUTH_TOKEN" in req.environ:
-            context['user'] = manager.AuthManager().get_user_from_access_key(
-                              req.environ['HTTP_X_AUTH_TOKEN'])
-            if context['user']:
-                context['project'] = manager.AuthManager().get_project(
-                                     context['user'].name)
-        if "user" not in context:
-            return webob.exc.HTTPForbidden()
-        req.environ['nova.context'] = context
-        return self.application
-
-
-class APIRouter(wsgi.Router):
-    """
-    Routes requests on the Rackspace API to the appropriate controller
-    and method.
-    """
-
-    def __init__(self):
-        mapper = routes.Mapper()
-
-        mapper.resource("server", "servers",
-                controller=controllers.ServersController())
-        mapper.resource("image", "images",
-                controller=controllers.ImagesController())
-        mapper.resource("flavor", "flavors",
-                controller=controllers.FlavorsController())
-        mapper.resource("sharedipgroup", "sharedipgroups",
-                controller=controllers.SharedIpGroupsController())
-
-        super(APIRouter, self).__init__(mapper)

nova/endpoint/rackspace/controllers/__init__.py

@@ -1,5 +0,0 @@
-from nova.endpoint.rackspace.controllers.images import ImagesController
-from nova.endpoint.rackspace.controllers.flavors import FlavorsController
-from nova.endpoint.rackspace.controllers.servers import ServersController
-from nova.endpoint.rackspace.controllers.sharedipgroups import \
-        SharedIpGroupsController

nova/endpoint/rackspace/controllers/base.py

@@ -1,9 +0,0 @@
-from nova import wsgi
-
-class BaseController(wsgi.Controller):
-    @classmethod
-    def render(cls, instance):
-        if isinstance(instance, list):
-            return { cls.entity_name : cls.render(instance) }
-        else:
-            return { "TODO": "TODO" }

nova/endpoint/rackspace/controllers/flavors.py

@@ -1 +0,0 @@
-class FlavorsController(object): pass

nova/endpoint/rackspace/controllers/images.py

@@ -1 +0,0 @@
-class ImagesController(object): pass

nova/endpoint/rackspace/controllers/servers.py

@@ -1,63 +0,0 @@
-from nova import rpc
-from nova.compute import model as compute
-from nova.endpoint.rackspace.controllers.base import BaseController
-
-class ServersController(BaseController):
-    entity_name = 'servers'
-
-    def index(self, **kwargs):
-        return [instance_details(inst) for inst in compute.InstanceDirectory().all]
-
-    def show(self, **kwargs):
-        instance_id = kwargs['id']
-        return compute.InstanceDirectory().get(instance_id)
-
-    def delete(self, **kwargs):
-        instance_id = kwargs['id']
-        instance = compute.InstanceDirectory().get(instance_id)
-        if not instance:
-            raise ServerNotFound("The requested server was not found")
-        instance.destroy()
-        return True
-
-    def create(self, **kwargs):
-        inst = self.build_server_instance(kwargs['server'])
-        rpc.cast(
-            FLAGS.compute_topic, {
-                "method": "run_instance",
-                "args": {"instance_id": inst.instance_id}})
-
-    def update(self, **kwargs):
-        instance_id = kwargs['id']
-        instance = compute.InstanceDirectory().get(instance_id)
-        if not instance:
-            raise ServerNotFound("The requested server was not found")
-        instance.update(kwargs['server'])
-        instance.save()
-
-    def build_server_instance(self, env):
-        """Build instance data structure and save it to the data store."""
-        reservation = utils.generate_uid('r')
-        ltime = time.strftime('%Y-%m-%dT%H:%M:%SZ', time.gmtime())
-        inst = self.instdir.new()
-        inst['name'] = env['server']['name']
-        inst['image_id'] = env['server']['imageId']
-        inst['instance_type'] = env['server']['flavorId']
-        inst['user_id'] = env['user']['id']
-        inst['project_id'] = env['project']['id']
-        inst['reservation_id'] = reservation
-        inst['launch_time'] = ltime
-        inst['mac_address'] = utils.generate_mac()
-        address = self.network.allocate_ip(
-                    inst['user_id'],
-                    inst['project_id'],
-                    mac=inst['mac_address'])
-        inst['private_dns_name'] = str(address)
-        inst['bridge_name'] = network.BridgedNetwork.get_network_for_project(
-                                inst['user_id'],
-                                inst['project_id'],
-                                'default')['bridge_name']
-        # key_data, key_name, ami_launch_index
-        # TODO(todd): key data or root password
-        inst.save()
-        return inst

nova/endpoint/rackspace/controllers/sharedipgroups.py

@@ -1 +0,0 @@
-class SharedIpGroupsController(object): pass

nova/process.py

@@ -2,6 +2,7 @@
 
 # Copyright 2010 United States Government as represented by the
 # Administrator of the National Aeronautics and Space Administration.
+# Copyright 2010 FathomDB Inc.
 # All Rights Reserved.
 #
 #    Licensed under the Apache License, Version 2.0 (the "License"); you may
@@ -20,17 +21,12 @@
 Process pool, still buggy right now.
 """
 
-import logging
-import multiprocessing
 import StringIO
 
 from twisted.internet import defer
 from twisted.internet import error
-from twisted.internet import process
 from twisted.internet import protocol
 from twisted.internet import reactor
-from twisted.internet import threads
-from twisted.python import failure
 
 from nova import flags
 
@@ -55,111 +51,100 @@ class UnexpectedErrorOutput(IOError):
         IOError.__init__(self, "got stdout: %r\nstderr: %r" % (stdout, stderr))
 
 
-# NOTE(termie): this too
+# This is based on _BackRelay from twister.internal.utils, but modified to 
-class _BackRelay(protocol.ProcessProtocol):
+#  capture both stdout and stderr, without odd stderr handling, and also to 
+#  handle stdin
+class BackRelayWithInput(protocol.ProcessProtocol):
     """
     Trivial protocol for communicating with a process and turning its output
     into the result of a L{Deferred}.
 
     @ivar deferred: A L{Deferred} which will be called back with all of stdout
-        and, if C{errortoo} is true, all of stderr as well (mixed together in
+        and all of stderr as well (as a tuple).  C{terminate_on_stderr} is true
-        one string).  If C{errortoo} is false and any bytes are received over
+        and any bytes are received over stderr, this will fire with an
-        stderr, this will fire with an L{_UnexpectedErrorOutput} instance and
+        L{_UnexpectedErrorOutput} instance and the attribute will be set to 
-        the attribute will be set to C{None}.
+        C{None}.
 
-    @ivar onProcessEnded: If C{errortoo} is false and bytes are received over
+    @ivar onProcessEnded: If C{terminate_on_stderr} is false and bytes are 
-        stderr, this attribute will refer to a L{Deferred} which will be called
+        received over stderr, this attribute will refer to a L{Deferred} which 
-        back when the process ends.  This C{Deferred} is also associated with
+        will be called back when the process ends.  This C{Deferred} is also 
-        the L{_UnexpectedErrorOutput} which C{deferred} fires with earlier in
+        associated with the L{_UnexpectedErrorOutput} which C{deferred} fires 
-        this case so that users can determine when the process has actually
+        with earlier in this case so that users can determine when the process 
-        ended, in addition to knowing when bytes have been received via stderr.
+        has actually ended, in addition to knowing when bytes have been received
+        via stderr.
     """
 
-    def __init__(self, deferred, errortoo=0):
+    def __init__(self, deferred, started_deferred=None, 
+                    terminate_on_stderr=False, check_exit_code=True, 
+                    process_input=None):
         self.deferred = deferred
-        self.s = StringIO.StringIO()
+        self.stdout = StringIO.StringIO()
-        if errortoo:
+        self.stderr = StringIO.StringIO()
-            self.errReceived = self.errReceivedIsGood
+        self.started_deferred = started_deferred
-        else:
+        self.terminate_on_stderr = terminate_on_stderr
-            self.errReceived = self.errReceivedIsBad
+        self.check_exit_code = check_exit_code
+        self.process_input = process_input
+        self.on_process_ended = None
     
-    def errReceivedIsBad(self, text):
+    def errReceived(self, text):
-        if self.deferred is not None:
+        self.stderr.write(text)
-            self.onProcessEnded = defer.Deferred()
+        if self.terminate_on_stderr and (self.deferred is not None):
-            err = UnexpectedErrorOutput(text, self.onProcessEnded)
+            self.on_process_ended = defer.Deferred()
-            self.deferred.errback(failure.Failure(err))
+            self.deferred.errback(UnexpectedErrorOutput(
+                                        stdout=self.stdout.getvalue(),
+                                        stderr=self.stderr.getvalue()))
             self.deferred = None
             self.transport.loseConnection()
 
-    def errReceivedIsGood(self, text):
-        self.s.write(text)
-
     def outReceived(self, text):
-        self.s.write(text)
+        self.stdout.write(text)
 
     def processEnded(self, reason):
         if self.deferred is not None:
-            self.deferred.callback(self.s.getvalue())
+            stdout, stderr = self.stdout.getvalue(), self.stderr.getvalue()
-        elif self.onProcessEnded is not None:
-            self.onProcessEnded.errback(reason)
-
-
-class BackRelayWithInput(_BackRelay):
-    def __init__(self, deferred, startedDeferred=None, error_ok=0,
-                 input=None):
-        # Twisted doesn't use new-style classes in most places :(
-        _BackRelay.__init__(self, deferred, errortoo=error_ok)
-        self.error_ok = error_ok
-        self.input = input
-        self.stderr = StringIO.StringIO()
-        self.startedDeferred = startedDeferred
-
-    def errReceivedIsBad(self, text):
-        self.stderr.write(text)
-        self.transport.loseConnection()
-
-    def errReceivedIsGood(self, text):
-        self.stderr.write(text)
-
-    def connectionMade(self):
-        if self.startedDeferred:
-            self.startedDeferred.callback(self)
-        if self.input:
-            self.transport.write(self.input)
-        self.transport.closeStdin()
-
-    def processEnded(self, reason):
-        if self.deferred is not None:
-            stdout, stderr = self.s.getvalue(), self.stderr.getvalue()
             try:
-                # NOTE(termie): current behavior means if error_ok is True
+                if self.check_exit_code:
-                #               we won't throw an error even if the process
-                #               exited with a non-0 status, so you can't be
-                #               okay with stderr output and not with bad exit
-                #               codes.
-                if not self.error_ok:
                     reason.trap(error.ProcessDone)
                 self.deferred.callback((stdout, stderr))
             except:
+                # NOTE(justinsb): This logic is a little suspicious to me...
+                # If the callback throws an exception, then errback will be 
+                # called also. However, this is what the unit tests test for...
                 self.deferred.errback(UnexpectedErrorOutput(stdout, stderr))
+        elif self.on_process_ended is not None:
+            self.on_process_ended.errback(reason)
 
 
-def getProcessOutput(executable, args=None, env=None, path=None, reactor=None,
+    def connectionMade(self):
-                     error_ok=0, input=None, startedDeferred=None):
+        if self.started_deferred:
-    if reactor is None:
+            self.started_deferred.callback(self)
-        from twisted.internet import reactor
+        if self.process_input:
+            self.transport.write(self.process_input)
+        self.transport.closeStdin()
+
+def get_process_output(executable, args=None, env=None, path=None, 
+                       process_reactor=None, check_exit_code=True, 
+                       process_input=None, started_deferred=None,
+                       terminate_on_stderr=False):
+    if process_reactor is None:
+        process_reactor = reactor
     args = args and args or ()
     env = env and env and {}
-    d = defer.Deferred()
+    deferred = defer.Deferred()
-    p = BackRelayWithInput(
+    process_handler = BackRelayWithInput(
-            d, startedDeferred=startedDeferred, error_ok=error_ok, input=input)
+            deferred, 
+            started_deferred=started_deferred, 
+            check_exit_code=check_exit_code, 
+            process_input=process_input,
+            terminate_on_stderr=terminate_on_stderr)
     # NOTE(vish): commands come in as unicode, but self.executes needs
     #             strings or process.spawn raises a deprecation warning
     executable = str(executable)
     if not args is None:
         args = [str(x) for x in args]
-    reactor.spawnProcess(p, executable, (executable,)+tuple(args), env, path)
+    process_reactor.spawnProcess(   process_handler, executable, 
-    return d
+                                    (executable,)+tuple(args), env, path)
+    return deferred
 
 
 class ProcessPool(object):
@@ -185,26 +170,26 @@ class ProcessPool(object):
         return self.execute(executable, args, **kw)
 
     def execute(self, *args, **kw):
-        d = self._pool.acquire()
+        deferred = self._pool.acquire()
 
-        def _associateProcess(proto):
+        def _associate_process(proto):
-            d.process = proto.transport
+            deferred.process = proto.transport
             return proto.transport
 
         started = defer.Deferred()
-        started.addCallback(_associateProcess)
+        started.addCallback(_associate_process)
-        kw.setdefault('startedDeferred', started)
+        kw.setdefault('started_deferred', started)
 
-        d.process = None
+        deferred.process = None
-        d.started = started
+        deferred.started = started
 
-        d.addCallback(lambda _: getProcessOutput(*args, **kw))
+        deferred.addCallback(lambda _: get_process_output(*args, **kw))
-        d.addBoth(self._release)
+        deferred.addBoth(self._release)
-        return d
+        return deferred
 
-    def _release(self, rv=None):
+    def _release(self, retval=None):
         self._pool.release()
-        return rv
+        return retval
 
 
 class SharedPool(object):

nova/rpc.py

@@ -59,7 +59,7 @@ class Connection(carrot_connection.BrokerConnection):
                 params['backend_cls'] = fakerabbit.Backend
 
             # NOTE(vish): magic is fun!
-            # pylint: disable=W0142
+            # pylint: disable-msg=W0142
             cls._instance = cls(**params)
         return cls._instance
 
@@ -104,7 +104,7 @@ class Consumer(messaging.Consumer):
             if self.failed_connection:
                 # NOTE(vish): conn is defined in the parent class, we can
                 #             recreate it as long as we create the backend too
-                # pylint: disable=W0201
+                # pylint: disable-msg=W0201
                 self.conn = Connection.recreate()
                 self.backend = self.conn.create_backend()
             super(Consumer, self).fetch(no_ack, auto_ack, enable_callbacks)
@@ -114,7 +114,7 @@ class Consumer(messaging.Consumer):
         # NOTE(vish): This is catching all errors because we really don't
         #             exceptions to be logged 10 times a second if some
         #             persistent failure occurs.
-        except Exception:  # pylint: disable=W0703
+        except Exception:  # pylint: disable-msg=W0703
             if not self.failed_connection:
                 logging.exception("Failed to fetch message from queue")
                 self.failed_connection = True
@@ -178,7 +178,7 @@ class AdapterConsumer(TopicConsumer):
         node_func = getattr(self.proxy, str(method))
         node_args = dict((str(k), v) for k, v in args.iteritems())
         # NOTE(vish): magic is fun!
-        # pylint: disable=W0142
+        # pylint: disable-msg=W0142
         d = defer.maybeDeferred(node_func, **node_args)
         if msg_id:
             d.addCallback(lambda rval: msg_reply(msg_id, rval, None))

nova/tests/network_unittest.py

@@ -36,7 +36,7 @@ FLAGS = flags.FLAGS
 
 class NetworkTestCase(test.TrialTestCase):
     """Test cases for network code"""
-    def setUp(self):  # pylint: disable=C0103
+    def setUp(self):  # pylint: disable-msg=C0103
         super(NetworkTestCase, self).setUp()
         # NOTE(vish): if you change these flags, make sure to change the
         #             flags in the corresponding section in nova-dhcpbridge
@@ -65,7 +65,7 @@ class NetworkTestCase(test.TrialTestCase):
                                          {'mac_address': utils.generate_mac()})
         self.instance2_id = instance_id
 
-    def tearDown(self):  # pylint: disable=C0103
+    def tearDown(self):  # pylint: disable-msg=C0103
         super(NetworkTestCase, self).tearDown()
         # TODO(termie): this should really be instantiating clean datastores
         #               in between runs, one failure kills all the tests
@@ -140,7 +140,6 @@ class NetworkTestCase(test.TrialTestCase):
         self.assertTrue(is_allocated_in_project(address2, self.projects[1].id))
 
         self.service.deallocate_fixed_ip(address2)
-        issue_ip(address2, net.bridge)
         release_ip(address2, net2.bridge)
         self.assertFalse(is_allocated_in_project(address2, self.projects[1].id))
 

nova/tests/process_unittest.py

@@ -48,7 +48,7 @@ class ProcessTestCase(test.TrialTestCase):
 
     def test_execute_stderr(self):
         pool = process.ProcessPool(2)
-        d = pool.simple_execute('cat BAD_FILE', error_ok=1)
+        d = pool.simple_execute('cat BAD_FILE', check_exit_code=False)
         def _check(rv):
             self.assertEqual(rv[0], '')
             self.assert_('No such file' in rv[1])

nova/tests/rpc_unittest.py

@@ -32,7 +32,7 @@ FLAGS = flags.FLAGS
 
 class RpcTestCase(test.BaseTestCase):
     """Test cases for rpc"""
-    def setUp(self):  # pylint: disable=C0103
+    def setUp(self):  # pylint: disable-msg=C0103
         super(RpcTestCase, self).setUp()
         self.conn = rpc.Connection.instance()
         self.receiver = TestReceiver()

nova/utils.py

@@ -56,23 +56,25 @@ def fetchfile(url, target):
 #    c.perform()
 #    c.close()
 #    fp.close()
-    execute("curl %s -o %s" % (url, target))
+    execute("curl --fail %s -o %s" % (url, target))
 
-
+def execute(cmd, process_input=None, addl_env=None, check_exit_code=True):
-def execute(cmd, input=None, addl_env=None):
     env = os.environ.copy()
     if addl_env:
         env.update(addl_env)
     obj = subprocess.Popen(cmd, shell=True, stdin=subprocess.PIPE,
         stdout=subprocess.PIPE, stderr=subprocess.PIPE, env=env)
     result = None
-    if input != None:
+    if process_input != None:
-        result = obj.communicate(input)
+        result = obj.communicate(process_input)
     else:
         result = obj.communicate()
     obj.stdin.close()
     if obj.returncode:
         logging.debug("Result was %s" % (obj.returncode))
+        if check_exit_code and obj.returncode <> 0:
+            raise Exception(    "Unexpected exit code: %s.  result=%s" 
+                                % (obj.returncode, result))
     return result
 
 
@@ -98,9 +100,13 @@ def debug(arg):
     return arg
 
 
-def runthis(prompt, cmd):
+def runthis(prompt, cmd, check_exit_code = True):
     logging.debug("Running %s" % (cmd))
-    logging.debug(prompt % (subprocess.call(cmd.split(" "))))
+    exit_code = subprocess.call(cmd.split(" "))
+    logging.debug(prompt % (exit_code))
+    if check_exit_code and exit_code <> 0:
+        raise Exception(    "Unexpected exit code: %s from cmd: %s" 
+                            % (exit_code, cmd))
 
 
 def generate_uid(topic, size=8):

nova/wsgi.py

@@ -83,7 +83,7 @@ class Application(object):
         raise NotImplementedError("You must implement __call__")
 
 
-class Middleware(Application): # pylint: disable=W0223
+class Middleware(Application):
     """
     Base WSGI middleware wrapper. These classes require an application to be
     initialized that will be called next.  By default the middleware will
@@ -91,11 +91,11 @@ class Middleware(Application): # pylint: disable=W0223
     behavior.
     """
 
-    def __init__(self, application): # pylint: disable=W0231
+    def __init__(self, application): # pylint: disable-msg=W0231
         self.application = application
 
     @webob.dec.wsgify
-    def __call__(self, req):
+    def __call__(self, req): # pylint: disable-msg=W0221
         """Override to implement middleware behavior."""
         return self.application
 
@@ -113,7 +113,7 @@ class Debug(Middleware):
         resp = req.get_response(self.application)
 
         print ("*" * 40) + " RESPONSE HEADERS"
-        for (key, value) in resp.headers:
+        for (key, value) in resp.headers.iteritems():
             print key, "=", value
         print
 
@@ -176,8 +176,9 @@ class Router(object):
         """
         return self._router
 
+    @staticmethod
     @webob.dec.wsgify
-    def _dispatch(self, req):
+    def _dispatch(req):
         """
         Called by self._router after matching the incoming request to a route
         and putting the information into req.environ.  Either returns 404
@@ -197,6 +198,7 @@ class Controller(object):
     must, in addition to their normal parameters, accept a 'req' argument
     which is the incoming webob.Request.
     """
+
     @webob.dec.wsgify
     def __call__(self, req):
         """
@@ -249,6 +251,7 @@ class Serializer(object):
             return repr(data)
 
     def _to_xml_node(self, doc, metadata, nodename, data):
+        """Recursive method to convert data members to XML nodes."""
         result = doc.createElement(nodename)
         if type(data) is list:
             singular = metadata.get('plurals', {}).get(nodename, None)

nova/wsgi_test.py

@@ -0,0 +1,96 @@
+# vim: tabstop=4 shiftwidth=4 softtabstop=4
+
+# Copyright 2010 United States Government as represented by the
+# Administrator of the National Aeronautics and Space Administration.
+# Copyright 2010 OpenStack LLC.
+# All Rights Reserved.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+"""
+Test WSGI basics and provide some helper functions for other WSGI tests.
+"""
+
+import unittest
+
+import routes
+import webob
+
+from nova import wsgi
+
+
+class Test(unittest.TestCase):
+
+    def test_debug(self):
+
+        class Application(wsgi.Application):
+            """Dummy application to test debug."""
+
+            def __call__(self, environ, start_response):
+                start_response("200", [("X-Test", "checking")])
+                return ['Test result']
+
+        application = wsgi.Debug(Application())
+        result = webob.Request.blank('/').get_response(application)
+        self.assertEqual(result.body, "Test result")
+
+    def test_router(self):
+
+        class Application(wsgi.Application):
+            """Test application to call from router."""
+
+            def __call__(self, environ, start_response):
+                start_response("200", [])
+                return ['Router result']
+
+        class Router(wsgi.Router):
+            """Test router."""
+
+            def __init__(self):
+                mapper = routes.Mapper()
+                mapper.connect("/test", controller=Application())
+                super(Router, self).__init__(mapper)
+
+        result = webob.Request.blank('/test').get_response(Router())
+        self.assertEqual(result.body, "Router result")
+        result = webob.Request.blank('/bad').get_response(Router())
+        self.assertNotEqual(result.body, "Router result")
+
+    def test_controller(self):
+
+        class Controller(wsgi.Controller):
+            """Test controller to call from router."""
+            test = self
+
+            def show(self, req, id): # pylint: disable-msg=W0622,C0103
+                """Default action called for requests with an ID."""
+                self.test.assertEqual(req.path_info, '/tests/123')
+                self.test.assertEqual(id, '123')
+                return id
+
+        class Router(wsgi.Router):
+            """Test router."""
+
+            def __init__(self):
+                mapper = routes.Mapper()
+                mapper.resource("test", "tests", controller=Controller())
+                super(Router, self).__init__(mapper)
+
+        result = webob.Request.blank('/tests/123').get_response(Router())
+        self.assertEqual(result.body, "123")
+        result = webob.Request.blank('/test/123').get_response(Router())
+        self.assertNotEqual(result.body, "123")
+
+    def test_serializer(self):
+        # TODO(eday): Placeholder for serializer testing.
+        pass

pylintrc

@@ -1,19 +1,26 @@
 [Messages Control]
-disable=C0103
+# W0511: TODOs in code comments are fine.
-# TODOs in code comments are fine...
+# W0142: *args and **kwargs are fine.
-disable=W0511
+disable-msg=W0511,W0142
-# *args and **kwargs are fine
-disable=W0142
 
 [Basic]
-# Variables can be 1 to 31 characters long, with
+# Variable names can be 1 to 31 characters long, with lowercase and underscores
-# lowercase and underscores
 variable-rgx=[a-z_][a-z0-9_]{0,30}$
 
+# Argument names can be 2 to 31 characters long, with lowercase and underscores
+argument-rgx=[a-z_][a-z0-9_]{1,30}$
+
 # Method names should be at least 3 characters long
 # and be lowecased with underscores
 method-rgx=[a-z_][a-z0-9_]{2,50}$
 
+# Module names matching nova-* are ok (files in bin/)
+module-rgx=(([a-z_][a-z0-9_]*)|([A-Z][a-zA-Z0-9]+)|(nova-[a-z0-9_-]+))$
+
+# Don't require docstrings on tests.
+no-docstring-rgx=((__.*__)|([tT]est.*)|setUp|tearDown)$
+
 [Design]
 max-public-methods=100
 min-public-methods=0
+max-args=6