launchpad-reviewers team mailing list archive

[Julian Edwards <julian.edwards@xxxxxxxxxxxxx>]

Julian Edwards has proposed merging lp:~julian-edwards/maas/backport-r1265-1267 into lp:maas/1.2.

Commit message:
Rearrange maas-cli's --help output, and add to the API docstrings (backport r1265, r1267 from trunk)

Requested reviews:
  MAAS Maintainers (maas-maintainers)
Related bugs:
  Bug #1085788 in MAAS: "Many API docstrings missing"
  https://bugs.launchpad.net/maas/+bug/1085788

For more details, see:
https://code.launchpad.net/~julian-edwards/maas/backport-r1265-1267/+merge/137473
-- 
https://code.launchpad.net/~julian-edwards/maas/backport-r1265-1267/+merge/137473
Your team MAAS Maintainers is requested to review the proposed merge of lp:~julian-edwards/maas/backport-r1265-1267 into lp:maas/1.2.

=== modified file 'src/maascli/api.py'
--- src/maascli/api.py	2012-10-08 20:56:17 +0000
+++ src/maascli/api.py	2012-12-03 04:59:21 +0000
@@ -21,6 +21,10 @@
 import json
 from operator import itemgetter
 import sys
+from textwrap import (
+    dedent,
+    fill,
+    )
 from urlparse import (
     urljoin,
     urlparse,
@@ -257,7 +261,8 @@
             }
         action_class = type(action_name, action_bases, action_ns)
         action_parser = parser.subparsers.add_parser(
-            action_name, help=help_title, description=help_body)
+            action_name, help=help_title, description=help_title,
+            epilog=help_body)
         action_parser.set_defaults(
             execute=action_class(action_parser))
 
@@ -267,7 +272,8 @@
     help_title, help_body = parse_docstring(handler["doc"])
     handler_name = handler_command_name(handler["name"])
     handler_parser = parser.subparsers.add_parser(
-        handler_name, help=help_title, description=help_body)
+        handler_name, help=help_title, description=help_title,
+        epilog=help_body)
     register_actions(profile, handler, handler_parser)
 
 
@@ -308,11 +314,32 @@
             register_handler(profile, represent_as, parser)
 
 
+profile_help = '\n\n'.join(
+    map(fill, map(dedent, [
+    """\
+        This is a profile.  Any commands you issue on this
+        profile will operate on the MAAS region server.
+        """,
+    """\
+        The command information you see here comes from the
+        region server's API; it may differ for different
+        profiles.  If you believe the API may have changed,
+        use the command's 'refresh' sub-command to fetch the
+        latest version of this help information from the
+        server.
+        """,
+    ])))
+
+
 def register_api_commands(parser):
     """Register all profiles as subcommands on `parser`."""
     with ProfileConfig.open() as config:
         for profile_name in config:
             profile = config[profile_name]
             profile_parser = parser.subparsers.add_parser(
-                profile["name"], help="Interact with %(url)s" % profile)
+                profile["name"], help="Interact with %(url)s" % profile,
+                description=(
+                    "Issue commands to the MAAS region controller at %(url)s."
+                    % profile),
+                epilog=profile_help)
             register_resources(profile, profile_parser)

=== modified file 'src/maascli/cli.py'
--- src/maascli/cli.py	2012-10-09 11:54:44 +0000
+++ src/maascli/cli.py	2012-12-03 04:59:21 +0000
@@ -29,7 +29,7 @@
 
 
 class cmd_login(Command):
-    """Log-in to a remote API, storing its description and credentials.
+    """Log in to a remote API, and remember its description and credentials.
 
     If credentials are not provided on the command-line, they will be prompted
     for interactively.
@@ -95,7 +95,12 @@
 
 
 class cmd_refresh(Command):
-    """Refresh the API descriptions of all profiles."""
+    """Refresh the API descriptions of all profiles.
+
+    This retrieves the latest version of the help information for each
+    profile.  Use it to update your maas-cli client's information after an
+    upgrade to the MAAS server.
+    """
 
     def __call__(self, options):
         with ProfileConfig.open() as config:
@@ -107,7 +112,11 @@
 
 
 class cmd_logout(Command):
-    """Log-out of a remote API, purging any stored credentials."""
+    """Log out of a remote API, purging any stored credentials.
+
+    This will remove the given profile from your maas-cli client.  You can
+    re-create it by logging in again later.
+    """
 
     def __init__(self, parser):
         super(cmd_logout, self).__init__(parser)
@@ -151,5 +160,6 @@
     for name, command in commands.items():
         help_title, help_body = parse_docstring(command)
         command_parser = parser.subparsers.add_parser(
-            safe_name(name), help=help_title, description=help_body)
+            safe_name(name), help=help_title, description=help_title,
+            epilog=help_body)
         command_parser.set_defaults(execute=command(command_parser))

=== modified file 'src/maascli/utils.py'
--- src/maascli/utils.py	2012-10-08 20:56:17 +0000
+++ src/maascli/utils.py	2012-12-03 04:59:21 +0000
@@ -39,6 +39,11 @@
 
 
 def parse_docstring(thing):
+    """Parse python docstring for `thing`.
+
+    Returns a tuple: (title, body).  As per docstring convention, title is
+    the docstring's first paragraph and body is the rest.
+    """
     is_string = isinstance(thing, basestring)
     doc = cleandoc(thing) if is_string else getdoc(thing)
     doc = empty if doc is None else doc

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-11-29 13:22:20 +0000
+++ src/maasserver/api.py	2012-12-03 04:59:21 +0000
@@ -472,7 +472,10 @@
 
 
 class NodeHandler(OperationsHandler):
-    """Manage individual Nodes."""
+    """Manage an individual Node.
+
+    The Node is identified by its system_id.
+    """
     create = None  # Disable create.
     model = Node
     fields = DISPLAYED_NODE_FIELDS
@@ -662,7 +665,7 @@
 
 
 class AnonNodesHandler(AnonymousOperationsHandler):
-    """Create Nodes."""
+    """Anonymous access to Nodes."""
     create = read = update = delete = None
     model = Node
     fields = DISPLAYED_NODE_FIELDS
@@ -741,7 +744,7 @@
 
 
 class NodesHandler(OperationsHandler):
-    """Manage collection of Nodes."""
+    """Manage the collection of all Nodes in the MAAS."""
     create = read = update = delete = None
     anonymous = AnonNodesHandler
 
@@ -893,10 +896,12 @@
 
 
 class NodeMacsHandler(OperationsHandler):
-    """
-    Manage all the MAC addresses linked to a Node / Create a new MAC address
-    for a Node.
-
+    """Manage MAC addresses for a given Node.
+
+    This is where you manage the MAC addresses linked to a Node, including
+    associating a new MAC address with the Node.
+
+    The Node is identified by its system_id.
     """
     update = delete = None
 
@@ -920,7 +925,11 @@
 
 
 class NodeMacHandler(OperationsHandler):
-    """Manage a MAC address linked to a Node."""
+    """Manage a MAC address.
+
+    The MAC address object is identified by the system_id for the Node it
+    is attached to, plus the MAC address itself.
+    """
     create = update = None
     fields = ('mac_address',)
     model = MACAddress
@@ -1041,7 +1050,7 @@
 
 
 class AnonNodeGroupsHandler(AnonymousOperationsHandler):
-    """Anon Node-groups API."""
+    """Anonymous access to NodeGroups."""
     create = read = update = delete = None
     fields = DISPLAYED_NODEGROUP_FIELDS
 
@@ -1134,7 +1143,7 @@
 
 
 class NodeGroupsHandler(OperationsHandler):
-    """Node-groups API."""
+    """Manage NodeGroups."""
     anonymous = AnonNodeGroupsHandler
     create = read = update = delete = None
     fields = DISPLAYED_NODEGROUP_FIELDS
@@ -1203,7 +1212,17 @@
 
 
 class NodeGroupHandler(OperationsHandler):
-    """Node-group API."""
+    """Manage a NodeGroup.
+
+    NodeGroup is the internal name for a cluster.
+
+    The NodeGroup is identified by its UUID, a random identifier that looks
+    something like:
+
+        5977f6ab-9160-4352-b4db-d71a99066c4f
+
+    Each NodeGroup has its own uuid.
+    """
 
     create = update = delete = None
     fields = DISPLAYED_NODEGROUP_FIELDS
@@ -1222,6 +1241,11 @@
 
     @operation(idempotent=False)
     def update_leases(self, request, uuid):
+        """Submit latest state of DHCP leases within the cluster.
+
+        The cluster controller calls this periodically to tell the region
+        controller about the IP addresses it manages.
+        """
         leases = get_mandatory_param(request.data, 'leases')
         nodegroup = get_object_or_404(NodeGroup, uuid=uuid)
         check_nodegroup_access(request, nodegroup)
@@ -1280,7 +1304,11 @@
 
 
 class NodeGroupInterfacesHandler(OperationsHandler):
-    """NodeGroupInterfaces API."""
+    """Manage NodeGroupInterfaces.
+
+    A NodeGroupInterface is a network interface attached to a cluster
+    controller, with its network properties.
+    """
     create = read = update = delete = None
     fields = DISPLAYED_NODEGROUP_FIELDS
 
@@ -1329,7 +1357,11 @@
 
 
 class NodeGroupInterfaceHandler(OperationsHandler):
-    """NodeGroupInterface API."""
+    """Manage a NodeGroupInterface.
+
+    A NodeGroupInterface is identified by the uuid for its NodeGroup, and
+    the name of the network interface it represents: "eth0" for example.
+    """
     create = delete = None
     fields = DISPLAYED_NODEGROUP_FIELDS
 
@@ -1432,7 +1464,13 @@
 
 
 class TagHandler(OperationsHandler):
-    """Manage individual Tags."""
+    """Manage individual Tags.
+
+    Tags are properties that can be associated with a Node and serve as
+    criteria for selecting and allocating nodes.
+
+    A Tag is identified by its name.
+    """
     create = None
     model = Tag
     fields = (