Merge lp:~cboylan/boots/boots_cleanup into lp:boots

[Clark Boylan]

[Clark Boylan (cboylan)]

I felt the need for some mostly thoughtless work so I started to write much needed docstrings and apply some of the suggestions made by pylint.

[David Rosenbaum (davidjrosenbaum)]

Looks good; I'll go ahead and merge this.

review: Approve

=== modified file 'boots.py'
--- boots.py	2010-02-14 22:25:48 +0000
+++ boots.py	2010-02-17 03:55:20 +0000
@@ -28,15 +28,21 @@
 # Contributor(s):
 #
 # ##### END LICENSE BLOCK #####
+"""This module is the entry point to boots. It builds a configuration and
+starts a boots console with that configuration."""
 
 from boots.app import client_config
 from boots.lib.console import Console
 
 def main():
+    """Entrance point to the boots executable.
+
+    Loads a boots configuration and starts the boots console with that
+    configuration."""
     configuration = client_config.ClientConfig()
     configuration.load()
     console = Console(configuration)
     console.main()
     
-if __name__=='__main__':
+if __name__ == "__main__":
     main()

=== modified file 'boots/api/api.py'
--- boots/api/api.py	2010-02-15 22:15:08 +0000
+++ boots/api/api.py	2010-02-17 03:55:20 +0000
@@ -27,6 +27,12 @@
 # Contributor(s):
 #
 # ##### END LICENSE BLOCK #####
+"""This module provides a high level API to SQL DB servers. It sits above
+the Drizzle dbapi providing boots with a simple and easy to use interface to
+drizzle.
+
+This API can be implemented for any dbapi implementation allowing boots to
+speak to a variety of DB servers."""
 
 from drizzle import db
 
@@ -49,6 +55,8 @@
         self._connection = None
 
     def connect(self):
+        """Connect to the database server whose details were provided through
+        the __init__ method."""
         self._connection = db.connect(
             host     =  self.hostname,  
             port     =  self.port,
@@ -57,11 +65,14 @@
             database =  self.auth.get('database', ""))
 
     def disconnect(self):
+        """Disconnect from the connected database server."""
         self._connection.close()
         self._connection = None
 
     @property
     def is_connected(self): 
+        """Property that is True when the Server is connected and False
+        otherwise."""
         return self._connection is not None
     
     def execute(self, query):

=== modified file 'boots/app/client_config.py'
--- boots/app/client_config.py	2010-02-07 09:29:31 +0000
+++ boots/app/client_config.py	2010-02-17 03:55:20 +0000
@@ -27,6 +27,9 @@
 # Contributor(s):
 #
 # ##### END LICENSE BLOCK #####
+"""This module provides a class that builds a boots client configuration. The
+configuration is built using a set of defaults, an rc file, and command line
+options/arguments."""
 
 import optparse
 import os
@@ -40,7 +43,6 @@
 
         This method performs no IO and after initializaion you must call load
         to populate a config."""
-
         # Specify default values for a client configuration.
         self._defaults = {"command":            None,
                           "database":           None,
@@ -64,7 +66,8 @@
         self.conf_file = conf_file_loc
 
         # Build cli option parser.
-        version_string = "{name} version {version}".format(name = info.NAME, version = info.VERSION)
+        version_string = "{name} version {version}".format(name = info.NAME,
+                                                           version = info.VERSION)
         self._cli_parser = optparse.OptionParser(version = version_string);
         self._cli_parser.add_option("-c", "--command",
                                     action = "store",
@@ -132,9 +135,11 @@
                                     help = "Specify max history file length")
  
     def __getitem__(self, key):
+        """Allows one to use the [] operator to get config items."""
         return self._dict[key]
 
     def __setitem__(self, key, value):
+        """Allows one to use the [] to set config items."""
         self._dict[key] = value
 
     @property
@@ -147,7 +152,6 @@
 
         Options specified in rc file overwrite defaults and cli options
         overwrite rc options."""
-
         from_file = self.get_file_conf(self.conf_file)
         from_cli = self.get_cli_conf()
         self._dict.update(from_file)
@@ -155,7 +159,6 @@
 
     def get_file_conf(self, filepath):
         """Read a configuration from the specified file. Return a dict."""
-
         file_dict = {}
         try:
             execfile(filepath, {}, file_dict)
@@ -169,7 +172,6 @@
 
     def get_cli_conf(self):
         """Read a configuration from the cli. Return a dict."""
-
         (options, args) = self._cli_parser.parse_args()
         tmp_options = vars(options)
         new_options = {}

=== modified file 'boots/app/info.py'
--- boots/app/info.py	2009-12-24 02:43:12 +0000
+++ boots/app/info.py	2010-02-17 03:55:20 +0000
@@ -27,6 +27,8 @@
 # Contributor(s):
 #
 # ##### END LICENSE BLOCK #####
+"""Module that contains information about the boots client. For example
+the name of the client and its version."""
 
 import os
 

=== modified file 'boots/lib/console.py'
--- boots/lib/console.py	2010-02-11 00:11:38 +0000
+++ boots/lib/console.py	2010-02-17 03:55:20 +0000
@@ -27,6 +27,8 @@
 # Contributor(s):
 #
 # ##### END LICENSE BLOCK #####
+"""This module provides the boots Console class. This class is the core
+driver of any boots client."""
 
 from boots.api import api
 from boots.api.nodes import node

=== modified file 'boots/lib/ui/generic.py'
--- boots/lib/ui/generic.py	2010-02-07 09:29:31 +0000
+++ boots/lib/ui/generic.py	2010-02-17 03:55:20 +0000
@@ -27,12 +27,17 @@
 # Contributor(s):
 #
 # ##### END LICENSE BLOCK #####
+"""This module provides a simple generic UI for boots. The UI is composed of
+two pieces, each classes, the StreamDriver and StreamPresenter."""
 
 import sys
 import StringIO
 
 class StreamDriver(object):
+    """StreamDrivers receive stream inputs to boots, stdin for example."""
     def __init__(self, console, stream, lingo):
+        """Initialize a StreamDriver giving it a link back to its parent
+        console, the stream to receive input on, and the lingo being used."""
         self.console = console
         self.stream = stream
         self.lingo = lingo
@@ -55,7 +60,10 @@
         pass
 
 class StreamPresenter(object):
+    """StreamPresenters write out boots results to streams, stdout for
+    example."""
     def __init__(self, stream):
+        """Initialize a StreamPresenter giving it the stream to write to."""
         self.stream = stream
         
     def present(self, row):

=== modified file 'boots/lib/ui/plain.py'
--- boots/lib/ui/plain.py	2010-02-14 21:13:24 +0000
+++ boots/lib/ui/plain.py	2010-02-17 03:55:20 +0000
@@ -27,6 +27,8 @@
 # Contributor(s):
 #
 # ##### END LICENSE BLOCK #####
+"""This module provides a 'plain' UI for boots. The UI is plain because it
+uses stdin and stdout to do its job."""
 
 import readline
 import sys
@@ -36,7 +38,14 @@
 from boots.lib.ui.generic import StdinDriver, StdoutPresenter
 
 class PlainUI(StdinDriver, StdoutPresenter):
+    """Class that provides a 'plain' UI for boots. Input is taken on stdin,
+    primary and secondary prompts are provided depending on context, and
+    results are printed to stdout. These results are printed as tables when
+    they are Server packets and as string interpretations otherwise."""
     def __init__(self, console, prompt1, prompt2, hist_length, hist_file, lingo):
+        """Initialize a UI giving it its parent console, primary prompt,
+        secondary prompt, maximum number of history lines, history file, and
+        lingo to be used."""
         self.metacommands = MetaCommands({
             '\use': self.switchlingo})
         self.console = console
@@ -52,20 +61,26 @@
         self.read_history()
 
     def read_history(self):
+        """Read existing history from the history file."""
         try:
             readline.read_history_file(self.hist_file)
         except IOError:
             pass
 
     def save_history(self):
+        """Save history from this session back to the history file."""
         # FIXME: commands with newlines are not properly being preserved
         readline.write_history_file(self.hist_file)
         
     def unload(self):
+        """'unload' a PlainUI saving its history."""
         self.save_history()
 
     def get_input(self):
-        """Gets user input from stdin via a prompt."""
+        """Gets user input from stdin via a prompt.
+        
+        On incomplete inputs print the secondary prompt instead of the
+        primary print."""
         while True:
             try:
                 line = raw_input(self.prompt1)
@@ -87,14 +102,34 @@
             except EOFError:
                 return
             
-    def set_prompt(self, prompt):
-        self.prompt = prompt
+    def set_prompt(self, prompt1 = None, prompt2 = None):
+        """Change the prompts during execution of a PlainUI."""
+        if prompt1 is None and prompt2 is None:
+            return
+        elif prompt2 is None:
+            self.prompt1 = prompt1
+        elif prompt1 is None:
+            self.prompt2 = prompt2
+        else:
+            self.prompt1 = prompt1
+            self.prompt2 = prompt2
 
     def present(self, result):
+        """Print the result provided as an argument.
+
+        If the result is a packet buffer the results until the last packet is
+        received. At that point print all of the buffered result rows in table
+        format. Non packet results are presented in their string representation
+        as received."""
         def padded(fields, widths):
+            """Utility function used to convert rows from tuples to table rows.
+            Results are returned as strings."""
             return "| {0} |\n".format(" | ".join(map(str.ljust, fields, widths)))
         
         def show_NULL(value):
+            """There is a 'bug' in the dbapi that does not convert NULL objects
+            to the string 'NULL'. This utility function performs that
+            conversion."""
             return value if value is not None else "NULL"
         
         if type(result) is dict and "__server_execute_sql_query" in result:
@@ -123,4 +158,6 @@
     #interface: Register the lingo used in main(), and register lingos
     #switched to here.
     def switchlingo(self, lingo):
+        """Switch the lingo used by the PlainUI during execution to the
+        lingo provided as an argument."""
         self.lingo = lingo

=== modified file 'setup.py'
--- setup.py	2010-02-14 22:32:23 +0000
+++ setup.py	2010-02-17 03:55:20 +0000
@@ -28,6 +28,7 @@
 # Contributor(s):
 #
 # ##### END LICENSE BLOCK #####
+"""This is the setup script for boots. It uses the python distutils."""
 
 from distutils.core import setup
 
@@ -41,7 +42,8 @@
 boots_app_packages = ["boots.app"]
 boots_lib_packages = ["boots.lib", "boots.lib.ui", "boots.lib.ui.components",
                       "boots.lib.lingos", "boots.lib.lingos.lisp"]
-boots_packages = ["boots"] + boots_api_packages + boots_app_packages + boots_lib_packages
+boots_packages = ["boots"] + boots_api_packages +\
+                 boots_app_packages + boots_lib_packages
 
 setup(name="boots",
       version=boots_version,

=== modified file 'tests/boots/api/api_tests.py'
--- tests/boots/api/api_tests.py	2010-02-15 22:15:08 +0000
+++ tests/boots/api/api_tests.py	2010-02-17 03:55:20 +0000
@@ -1,10 +1,17 @@
+"""This module contains unit tests for the boots' api module."""
 import unittest
 from drizzle import errors
 from boots.api import api
 import boots_unit_test
 
 class TestServerConnections(boots_unit_test.BootsQueryTester):
+    """Class that provides unit tests that test the api.Server class' ability
+    to connect to servers as well as provide proper errors when connections
+    should fail."""
+    #FIXME should use the configuration provided.
     def setUp(self):
+        #We want to derive from the BootsQueryTester but not use the default
+        #setUp method due to the cost incurred by it.
         pass
 
     def test_valid_connect_fqdn(self):
@@ -44,13 +51,15 @@
         self.assert_(not server.is_connected)
 
     def test_invalid_disconnect(self):
-        # Todo this should probably be set up to assert an exception.
         server = api.Server("kororaa.cs.pdx.edu", 9306, {})
         self.assertRaises(errors.InterfaceError, server.connect)
         self.assertRaises(AttributeError, server.disconnect)
         self.assert_(not server.is_connected)
 
 class TestServerExecute(boots_unit_test.BootsQueryTester):
+    """Class that provides unit tests that tests api.Server class' ability
+    to execute SQL statements on a server."""
+    #FIXME needs to test more than just SELECT statements.
     def setUp(self):
         boots_unit_test.BootsQueryTester.setUp(self)
         self.server = api.Server(self.config["host"], self.config["port"], {"database": self.config["database"]})
@@ -60,6 +69,10 @@
         self.server.disconnect()
 
     def _concat_packet_results(self, iter):
+        """Results come from the Server object in 'packets'. These packets need
+        their rows extracted so that they can be compared properly. This method
+        takes an iterator of packets and does this extraction for you. Results
+        are returned in a single list."""
         result = []
         for packet in iter:
             result.extend(packet["result"])

=== modified file 'tests/boots/boots_unit_test.py'
--- tests/boots/boots_unit_test.py	2010-02-15 22:15:08 +0000
+++ tests/boots/boots_unit_test.py	2010-02-17 03:55:20 +0000
@@ -1,18 +1,67 @@
+# Boots Client
+# www.launchpad.net/boots
+# www.launchpad.net/drizzle
+# tests/boots/boots_unit_test.py
+#
+# ##### BEGIN LICENSE BLOCK #####
+# Version: MPL 1.1
+#
+# The contents of this file are subject to the Mozilla Public License
+# Version 1.1 (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.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS IS"
+# basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See the
+# License for the specific language governing rights and limitations
+# under the License.
+#
+# The Original Code is Boots Client code.
+#
+# The Initial Developer of the Original Code is Clark Boylan, Ken
+# Brotherton, Max Goodman, Victoria Lewis, David Rosenbaum, and Andreas
+# Turriff. Portions created by Clark Boylan, Ken Brotherton, Max Goodman,
+# Victoria Lewis, David Rosenbaum, and Andreas Turriff are Copyright (C)
+# 2009. All Rights Reserved.
+#
+# Contributor(s):
+#
+# ##### END LICENSE BLOCK #####
+"""This module contains the base test classes to be used for boots unit
+tests. BootsBaseTester contains the test config. BootsQueryTester includes
+the config and its setUp method will load a fresh test DB for testing."""
+
 import unittest
 import os.path
 from subprocess import Popen, PIPE
 
 class BootsBaseTester(unittest.TestCase):
+    """Simple unit test test case class that includes a boots testing config.
+
+    All boots unit test classes should inherit from this class or one of its
+    children."""
     config = {"host": "localhost",
               "port": 9306,
               "database": "boots_fec_test"}
 
 class BootsQueryTester(BootsBaseTester):
+    """Unit test class that includes a setUp method that can be used to load
+    a fresh copy of the test database before each unit test.
+
+    Do not use this class unless you need a fresh database loaded into the
+    server before each test. The time cost of doing this is quite high and
+    should be avoided whenever possible. Additionally the current drizzle
+    client should be in your path when you run tests using this class."""
     def setUp(self):
+        """Default setUp method that loads a fresh copy of the test DB into
+        the server before each test.
+
+        This allows any classes deriving from this one to easily get clean test
+        data for its tests."""
         # The drizzle client should be in your path allowing the DB to be
         # created if necessary.
-        cmd = "drizzle -h {host} -p {port} -B < "\
-              .format(**self.config) + os.path.dirname(os.path.realpath(__file__)) + "/fec.sql"
+        cmd = "drizzle -h {host} -p {port} -B < ".format(**self.config) +\
+              os.path.dirname(os.path.realpath(__file__)) + "/fec.sql"
         p = Popen(cmd, shell=True, stdin=PIPE, stdout=PIPE, close_fds=True)
         (cin, cout) = (p.stdin, p.stdout)
         cin.close()

=== modified file 'tests/boots/tests.py'
--- tests/boots/tests.py	2010-02-15 22:15:08 +0000
+++ tests/boots/tests.py	2010-02-17 03:55:20 +0000
@@ -1,11 +1,46 @@
+#!/usr/bin/env python
+# Boots Client
+# www.launchpad.net/boots
+# www.launchpad.net/drizzle
+# tests/boots/tests.py
+#
+# ##### BEGIN LICENSE BLOCK #####
+# Version: MPL 1.1
+#
+# The contents of this file are subject to the Mozilla Public License
+# Version 1.1 (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.mozilla.org/MPL/
+#
+# Software distributed under the License is distributed on an "AS IS"
+# basis, WITHOUT WARRANTY OF ANY KIND, either express or implied. See the
+# License for the specific language governing rights and limitations
+# under the License.
+#
+# The Original Code is Boots Client code.
+#
+# The Initial Developer of the Original Code is Clark Boylan, Ken
+# Brotherton, Max Goodman, Victoria Lewis, David Rosenbaum, and Andreas
+# Turriff. Portions created by Clark Boylan, Ken Brotherton, Max Goodman,
+# Victoria Lewis, David Rosenbaum, and Andreas Turriff are Copyright (C)
+# 2009. All Rights Reserved.
+#
+# Contributor(s):
+#
+# ##### END LICENSE BLOCK #####
+"""This module contains the boots test driver. The test driver runs all tests by
+default or any specified on the command line."""
+
 import unittest
 from optparse import OptionParser
 import sys
 import os
 import os.path
 from boots_unit_test import BootsBaseTester
+
 # Path mangling to import boots files properly.
-new_path = os.path.dirname(os.path.dirname(os.path.dirname(os.path.realpath(__file__))))
+new_path = os.path.dirname(os.path.dirname(os.path.dirname(
+           os.path.realpath(__file__))))
 sys.path.insert(0, new_path)
 from api import api_tests
 from api.nodes import node_tests
@@ -14,21 +49,30 @@
 from lib.lingos import lingo_tests
 from lib.ui import ui_tests
 
-if __name__ == "__main__":
+def boots_test_driver():
+    """Function that reads test options from the command line and runs the
+    appropriate tests."""
     parser = OptionParser()
-    parser.add_option("-H", "--host", dest="host", default=BootsBaseTester.config["host"],
+    parser.add_option("-H", "--host", dest="host",
+                      default=BootsBaseTester.config["host"],
                       help="Host running the Drizzle server on which the test\
                             DB should be run.")
-    parser.add_option("-p", "--port", type="int", dest="port", default=BootsBaseTester.config["port"],
+    parser.add_option("-p", "--port", type="int", dest="port",
+                      default=BootsBaseTester.config["port"],
                       help="Port the Drizzle server is running on.")
-    parser.add_option("-D", "--database", dest="database", default=BootsBaseTester.config["database"],
+    parser.add_option("-D", "--database", dest="database",
+                      default=BootsBaseTester.config["database"],
                       help="Database to use for testing on the Drizzle server.")
     (options, args) = parser.parse_args()
     BootsBaseTester.config.update(vars(options))
     if not args:
-        modules = [api_tests, node_tests, app_tests, console_tests, lingo_tests, ui_tests]
+        modules = [api_tests, node_tests, app_tests,
+                   console_tests, lingo_tests, ui_tests]
     else:
         modules = map(globals().__getitem__, args)
     for module in modules:
         suite = unittest.TestLoader().loadTestsFromModule(module)
         unittest.TextTestRunner(verbosity=2).run(suite)
+
+if __name__ == "__main__":
+    boots_test_driver()