docstring cleanup, direct api, part of compute

nova/api/__init__.py

@@ -15,5 +15,3 @@
 #    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
 #    License for the specific language governing permissions and limitations
 #    under the License.
-
-"""No-op __init__ for directory full of api goodies."""

nova/api/direct.py

@@ -44,14 +44,33 @@ from nova import utils
 from nova import wsgi
 
 
+# Global storage for registering modules.
 ROUTES = {}
 
 
 def register_service(path, handle):
+    """Register a service handle at a given path.
+
+    Services registered in this way will be made available to any instances of
+    nova.api.direct.Router.
+
+    :param path: `routes` path, can be a basic string like "/path"
+    :param handle: an object whose methods will be made available via the api
+
+    """
     ROUTES[path] = handle
 
 
 class Router(wsgi.Router):
+    """A simple WSGI router configured via `register_service`.
+
+    This is a quick way to attach multiple services to a given endpoint.
+    It will automatically load the routes registered in the `ROUTES` global.
+
+    TODO(termie): provide a paste-deploy version of this.
+
+    """
+
     def __init__(self, mapper=None):
         if mapper is None:
             mapper = routes.Mapper()
@@ -66,6 +85,24 @@ class Router(wsgi.Router):
 
 
 class DelegatedAuthMiddleware(wsgi.Middleware):
+    """A simple and naive authentication middleware.
+
+    Designed mostly to provide basic support for alternative authentication
+    schemes, this middleware only desires the identity of the user and will
+    generate the appropriate nova.context.RequestContext for the rest of the
+    application. This allows any middleware above it in the stack to
+    authenticate however it would like while only needing to conform to a
+    minimal interface.
+
+    Expects two headers to determine identity:
+     - X-OpenStack-User
+     - X-OpenStack-Project
+
+    This middleware is tied to identity management and will need to be kept
+    in sync with any changes to the way identity is dealt with internally.
+
+    """
+
     def process_request(self, request):
         os_user = request.headers['X-OpenStack-User']
         os_project = request.headers['X-OpenStack-Project']
@@ -74,6 +111,20 @@ class DelegatedAuthMiddleware(wsgi.Middleware):
 
 
 class JsonParamsMiddleware(wsgi.Middleware):
+    """Middleware to allow method arguments to be passed as serialized JSON.
+
+    Accepting arguments as JSON is useful for accepting data that may be more
+    complex than simple primitives.
+
+    In this case we accept it as urlencoded data under the key 'json' as in
+    json=<urlencoded_json> but this could be extended to accept raw JSON
+    in the POST body.
+
+    Filters out the parameters `self`, `context` and anything beginning with
+    an underscore.
+
+    """
+
     def process_request(self, request):
         if 'json' not in request.params:
             return
@@ -92,6 +143,13 @@ class JsonParamsMiddleware(wsgi.Middleware):
 
 
 class PostParamsMiddleware(wsgi.Middleware):
+    """Middleware to allow method arguments to be passed as POST parameters.
+
+    Filters out the parameters `self`, `context` and anything beginning with
+    an underscore.
+
+    """
+
     def process_request(self, request):
         params_parsed = request.params
         params = {}
@@ -106,12 +164,21 @@ class PostParamsMiddleware(wsgi.Middleware):
 
 
 class Reflection(object):
-    """Reflection methods to list available methods."""
+    """Reflection methods to list available methods.
+
+    This is an object that expects to be registered via register_service.
+    These methods allow the endpoint to be self-describing. They introspect
+    the exposed methods and provide call signatures and documentation for
+    them allowing quick experimentation.
+
+    """
+
     def __init__(self):
         self._methods = {}
         self._controllers = {}
 
     def _gather_methods(self):
+        """Introspect available methods and generate documentation for them."""
         methods = {}
         controllers = {}
         for route, handler in ROUTES.iteritems():
@@ -185,6 +252,16 @@ class Reflection(object):
 
 
 class ServiceWrapper(wsgi.Controller):
+    """Wrapper to dynamically povide a WSGI controller for arbitrary objects.
+
+    With lightweight introspection allows public methods on the object to
+    be accesed via simple WSGI routing and parameters and serializes the
+    return values.
+
+    Automatically used be nova.api.direct.Router to wrap registered instances.
+
+    """
+
     def __init__(self, service_handle):
         self.service_handle = service_handle
 
@@ -260,7 +337,16 @@ class Limited(object):
 
 
 class Proxy(object):
-    """Pretend a Direct API endpoint is an object."""
+    """Pretend a Direct API endpoint is an object.
+
+    This is mostly useful in testing at the moment though it should be easily
+    extendable to provide a basic API library functionality.
+
+    In testing we use this to stub out internal objects to verify that results
+    from the API are serializable.
+
+    """
+
     def __init__(self, app, prefix=None):
         self.app = app
         self.prefix = prefix

nova/compute/api.py

@@ -16,9 +16,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-"""
+"""Handles all requests relating to instances (guest vms)."""
-Handles all requests relating to instances (guest vms).
-"""
 
 import datetime
 import re
@@ -86,10 +84,10 @@ class API(base.Base):
                         {"method": "get_network_topic", "args": {'fake': 1}})
 
     def _check_injected_file_quota(self, context, injected_files):
-        """
+        """Enforce quota limits on injected files.
-        Enforce quota limits on injected files
+
+        Raises a QuotaError if any limit is exceeded.
 
-        Raises a QuotaError if any limit is exceeded
         """
         if injected_files is None:
             return
@@ -111,8 +109,11 @@ class API(base.Base):
                key_name=None, key_data=None, security_group='default',
                availability_zone=None, user_data=None, metadata=[],
                injected_files=None):
-        """Create the number of instances requested if quota and
+        """Create the number and type of instances requested.
-        other arguments check out ok."""
+
+        Verifies that quota and other arguments are valid.
+
+        """
 
         if not instance_type:
             instance_type = instance_types.get_default_instance_type()
@@ -262,8 +263,7 @@ class API(base.Base):
         return [dict(x.iteritems()) for x in instances]
 
     def has_finished_migration(self, context, instance_id):
-        """Retrieves whether or not a finished migration exists for
+        """Returns true if an instance has a finished migration."""
-        an instance"""
         try:
             db.migration_get_by_instance_and_status(context, instance_id,
                     'finished')
@@ -272,8 +272,10 @@ class API(base.Base):
             return False
 
     def ensure_default_security_group(self, context):
-        """ Create security group for the security context if it
+        """Ensure that a context has a security group.
-        does not already exist
+
+        Creates a security group for the security context if it does not
+        already exist.
 
         :param context: the security context
 
@@ -289,7 +291,7 @@ class API(base.Base):
             db.security_group_create(context, values)
 
     def trigger_security_group_rules_refresh(self, context, security_group_id):
-        """Called when a rule is added to or removed from a security_group"""
+        """Called when a rule is added to or removed from a security_group."""
 
         security_group = self.db.security_group_get(context, security_group_id)
 
@@ -305,11 +307,12 @@ class API(base.Base):
                       "args": {"security_group_id": security_group.id}})
 
     def trigger_security_group_members_refresh(self, context, group_id):
-        """Called when a security group gains a new or loses a member
+        """Called when a security group gains a new or loses a member.
 
         Sends an update request to each compute node for whom this is
-        relevant."""
+        relevant.
 
+        """
         # First, we get the security group rules that reference this group as
         # the grantee..
         security_group_rules = \
@@ -354,7 +357,7 @@ class API(base.Base):
                        as data fields of the instance to be
                        updated
 
-        :retval None
+        :returns: None
 
         """
         rv = self.db.instance_update(context, instance_id, kwargs)
@@ -362,6 +365,7 @@ class API(base.Base):
 
     @scheduler_api.reroute_compute("delete")
     def delete(self, context, instance_id):
+        """Terminate an instance."""
         LOG.debug(_("Going to try to terminate %s"), instance_id)
         try:
             instance = self.get(context, instance_id)
@@ -393,22 +397,28 @@ class API(base.Base):
             self.db.instance_destroy(context, instance_id)
 
     def get(self, context, instance_id):
-        """Get a single instance with the given ID."""
+        """Get a single instance with the given instance_id."""
         rv = self.db.instance_get(context, instance_id)
         return dict(rv.iteritems())
 
     @scheduler_api.reroute_compute("get")
     def routing_get(self, context, instance_id):
-        """Use this method instead of get() if this is the only
+        """A version of get with special routing characteristics.
-           operation you intend to to. It will route to novaclient.get
+
-           if the instance is not found."""
+        Use this method instead of get() if this is the only operation you
+        intend to to. It will route to novaclient.get if the instance is not
+        found.
+
+        """
         return self.get(context, instance_id)
 
     def get_all(self, context, project_id=None, reservation_id=None,
                 fixed_ip=None):
-        """Get all instances, possibly filtered by one of the
+        """Get all instances filtered by one of the given parameters.
-        given parameters. If there is no filter and the context is
+
-        an admin, it will retreive all instances in the system.
+        If there is no filter and the context is an admin, it will retreive
+        all instances in the system.
+
         """
         if reservation_id is not None:
             return self.db.instance_get_all_by_reservation(
@@ -437,7 +447,8 @@ class API(base.Base):
         :param params: Optional dictionary of arguments to be passed to the
                        compute worker
 
-        :retval None
+        :returns: None
+
         """
         if not params:
             params = {}
@@ -456,7 +467,7 @@ class API(base.Base):
         :param params: Optional dictionary of arguments to be passed to the
                        compute worker
 
-        :retval: Result returned by compute worker
+        :returns: Result returned by compute worker
         """
         if not params:
             params = {}
@@ -469,13 +480,14 @@ class API(base.Base):
         return rpc.call(context, queue, kwargs)
 
     def _cast_scheduler_message(self, context, args):
-        """Generic handler for RPC calls to the scheduler"""
+        """Generic handler for RPC calls to the scheduler."""
         rpc.cast(context, FLAGS.scheduler_topic, args)
 
     def snapshot(self, context, instance_id, name):
         """Snapshot the given instance.
 
-        :retval: A dict containing image metadata
+        :returns: A dict containing image metadata
+
         """
         properties = {'instance_id': str(instance_id),
                       'user_id': str(context.user_id)}
@@ -492,7 +504,7 @@ class API(base.Base):
         self._cast_compute_message('reboot_instance', context, instance_id)
 
     def revert_resize(self, context, instance_id):
-        """Reverts a resize, deleting the 'new' instance in the process"""
+        """Reverts a resize, deleting the 'new' instance in the process."""
         context = context.elevated()
         migration_ref = self.db.migration_get_by_instance_and_status(context,
                 instance_id, 'finished')
@@ -507,8 +519,7 @@ class API(base.Base):
                 {'status': 'reverted'})
 
     def confirm_resize(self, context, instance_id):
-        """Confirms a migration/resize, deleting the 'old' instance in the
+        """Confirms a migration/resize and deletes the 'old' instance."""
-        process."""
         context = context.elevated()
         migration_ref = self.db.migration_get_by_instance_and_status(context,
                 instance_id, 'finished')
@@ -568,10 +579,9 @@ class API(base.Base):
     @scheduler_api.reroute_compute("diagnostics")
     def get_diagnostics(self, context, instance_id):
         """Retrieve diagnostics for the given instance."""
-        return self._call_compute_message(
+        return self._call_compute_message("get_diagnostics",
-            "get_diagnostics",
+                                          context,
-            context,
+                                          instance_id)
-            instance_id)
 
     def get_actions(self, context, instance_id):
         """Retrieve actions for the given instance."""
@@ -579,12 +589,12 @@ class API(base.Base):
 
     @scheduler_api.reroute_compute("suspend")
     def suspend(self, context, instance_id):
-        """suspend the instance with instance_id"""
+        """Suspend the given instance."""
         self._cast_compute_message('suspend_instance', context, instance_id)
 
     @scheduler_api.reroute_compute("resume")
     def resume(self, context, instance_id):
-        """resume the instance with instance_id"""
+        """Resume the given instance."""
         self._cast_compute_message('resume_instance', context, instance_id)
 
     @scheduler_api.reroute_compute("rescue")
@@ -599,15 +609,15 @@ class API(base.Base):
 
     def set_admin_password(self, context, instance_id, password=None):
         """Set the root/admin password for the given instance."""
-        self._cast_compute_message('set_admin_password', context, instance_id,
+        self._cast_compute_message(
-                                    password)
+                'set_admin_password', context, instance_id, password)
 
     def inject_file(self, context, instance_id):
         """Write a file to the given instance."""
         self._cast_compute_message('inject_file', context, instance_id)
 
     def get_ajax_console(self, context, instance_id):
-        """Get a url to an AJAX Console"""
+        """Get a url to an AJAX Console."""
         output = self._call_compute_message('get_ajax_console',
                                             context,
                                             instance_id)
@@ -616,7 +626,7 @@ class API(base.Base):
                   'args': {'token': output['token'], 'host': output['host'],
                   'port': output['port']}})
         return {'url': '%s/?token=%s' % (FLAGS.ajax_console_proxy_url,
-                output['token'])}
+                                         output['token'])}
 
     def get_vnc_console(self, context, instance_id):
         """Get a url to a VNC Console."""
@@ -638,39 +648,34 @@ class API(base.Base):
                        'portignore')}
 
     def get_console_output(self, context, instance_id):
-        """Get console output for an an instance"""
+        """Get console output for an an instance."""
         return self._call_compute_message('get_console_output',
                                           context,
                                           instance_id)
 
     def lock(self, context, instance_id):
-        """lock the instance with instance_id"""
+        """Lock the given instance."""
         self._cast_compute_message('lock_instance', context, instance_id)
 
     def unlock(self, context, instance_id):
-        """unlock the instance with instance_id"""
+        """Unlock the given instance."""
         self._cast_compute_message('unlock_instance', context, instance_id)
 
     def get_lock(self, context, instance_id):
-        """return the boolean state of (instance with instance_id)'s lock"""
+        """Return the boolean state of given instance's lock."""
         instance = self.get(context, instance_id)
         return instance['locked']
 
     def reset_network(self, context, instance_id):
-        """
+        """Reset networking on the instance."""
-        Reset networking on the instance.
-
-        """
         self._cast_compute_message('reset_network', context, instance_id)
 
     def inject_network_info(self, context, instance_id):
-        """
+        """Inject network info for the instance."""
-        Inject network info for the instance.
-
-        """
         self._cast_compute_message('inject_network_info', context, instance_id)
 
     def attach_volume(self, context, instance_id, volume_id, device):
+        """Attach an existing volume to an existing instance."""
         if not re.match("^/dev/[a-z]d[a-z]+$", device):
             raise exception.ApiError(_("Invalid device specified: %s. "
                                      "Example device: /dev/vdb") % device)
@@ -685,6 +690,7 @@ class API(base.Base):
                            "mountpoint": device}})
 
     def detach_volume(self, context, volume_id):
+        """Detach a volume from an instance."""
         instance = self.db.volume_get_instance(context.elevated(), volume_id)
         if not instance:
             raise exception.ApiError(_("Volume isn't attached to anything!"))
@@ -698,6 +704,7 @@ class API(base.Base):
         return instance
 
     def associate_floating_ip(self, context, instance_id, address):
+        """Associate a floating ip with an instance."""
         instance = self.get(context, instance_id)
         self.network_api.associate_floating_ip(context,
                                                floating_ip=address,
@@ -709,11 +716,11 @@ class API(base.Base):
         return dict(rv.iteritems())
 
     def delete_instance_metadata(self, context, instance_id, key):
-        """Delete the given metadata item"""
+        """Delete the given metadata item from an instance."""
         self.db.instance_metadata_delete(context, instance_id, key)
 
     def update_or_create_instance_metadata(self, context, instance_id,
                                             metadata):
-        """Updates or creates instance metadata"""
+        """Updates or creates instance metadata."""
         self.db.instance_metadata_update_or_create(context, instance_id,
                                                     metadata)

nova/compute/instance_types.py

@@ -18,9 +18,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-"""
+"""Built-in instance properties."""
-The built-in instance properties.
-"""
 
 from nova import context
 from nova import db
@@ -34,9 +32,7 @@ LOG = logging.getLogger('nova.instance_types')
 
 def create(name, memory, vcpus, local_gb, flavorid, swap=0,
            rxtx_quota=0, rxtx_cap=0):
-    """Creates instance types / flavors
+    """Creates instance types."""
-       arguments: name memory vcpus local_gb flavorid swap rxtx_quota rxtx_cap
-    """
     for option in [memory, vcpus, local_gb, flavorid]:
         try:
             int(option)
@@ -64,8 +60,7 @@ def create(name, memory, vcpus, local_gb, flavorid, swap=0,
 
 
 def destroy(name):
-    """Marks instance types / flavors as deleted
+    """Marks instance types as deleted."""
-    arguments: name"""
     if name == None:
         raise exception.InvalidInputException(_("No instance type specified"))
     else:
@@ -77,8 +72,7 @@ def destroy(name):
 
 
 def purge(name):
-    """Removes instance types / flavors from database
+    """Removes instance types from database."""
-    arguments: name"""
     if name == None:
         raise exception.InvalidInputException(_("No instance type specified"))
     else:
@@ -90,18 +84,19 @@ def purge(name):
 
 
 def get_all_types(inactive=0):
-    """Retrieves non-deleted instance_types.
+    """Get all non-deleted instance_types.
-    Pass true as argument if you want deleted instance types returned also."""
+
+    Pass true as argument if you want deleted instance types returned also.
+
+    """
     return db.instance_type_get_all(context.get_admin_context(), inactive)
 
 
-def get_all_flavors():
+get_all_flavors = get_all_types
-    """retrieves non-deleted flavors. alias for instance_types.get_all_types().
-    Pass true as argument if you want deleted instance types returned also."""
-    return get_all_types(context.get_admin_context())
 
 
 def get_default_instance_type():
+    """Get the default instance type."""
     name = FLAGS.default_instance_type
     try:
         return get_instance_type_by_name(name)
@@ -110,7 +105,7 @@ def get_default_instance_type():
 
 
 def get_instance_type(id):
-    """Retrieves single instance type by id"""
+    """Retrieves single instance type by id."""
     if id is None:
         return get_default_instance_type()
     try:
@@ -121,7 +116,7 @@ def get_instance_type(id):
 
 
 def get_instance_type_by_name(name):
-    """Retrieves single instance type by name"""
+    """Retrieves single instance type by name."""
     if name is None:
         return get_default_instance_type()
     try:
@@ -131,8 +126,10 @@ def get_instance_type_by_name(name):
         raise exception.ApiError(_("Unknown instance type: %s") % name)
 
 
+# TODO(termie): flavor-specific code should probably be in the API that uses
+#               flavors.
 def get_instance_type_by_flavor_id(flavor_id):
-    """retrieve instance type by flavor_id"""
+    """Retrieve instance type by flavor_id."""
     if flavor_id is None:
         return get_default_instance_type()
     try: