Merge lp:~javier.collado/utah/utah_client_common_docs into lp:utah

[Javier Collado]

This branch add some text to the documentation strings in utah.client.common
module.

[Joe Talbott (joetalbott)]

Looks good. Thanks.

review: Approve

=== modified file 'docs/source/reference.rst'
--- docs/source/reference.rst	2012-12-12 20:58:59 +0000
+++ docs/source/reference.rst	2013-02-27 09:07:24 +0000
@@ -59,6 +59,9 @@
 
 .. automodule:: utah.client
 
+``utah.client.common``
+----------------------
+
 .. automodule:: utah.client.common
    :members:
 

=== modified file 'utah/client/common.py'
--- utah/client/common.py	2013-02-15 08:55:48 +0000
+++ utah/client/common.py	2013-02-27 09:07:24 +0000
@@ -13,9 +13,7 @@
 # You should have received a copy of the GNU General Public License along
 # with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-"""
-Common methods.
-"""
+"""UTAH client common classes and functions."""
 
 import datetime
 import re
@@ -75,16 +73,39 @@
 
 
 def do_nothing(_obj=None):
-    """
-    Do nothing method used a placeholder for save_state_callbacks.
-    """
-    pass
+    """Placeholder for save_state_callbacks.
+
+    .. seealso:: :class:`.TestSuite`, :class:`.TestCase`
+
+    """
 
 
 # Inspired by:
 # http://stackoverflow.com/a/3326559
 def run_cmd(command, cwd=None, timeout=0, cmd_type=CMD_TC_TEST, run_as=None,
             battery_measurements=False):
+    """Run command and return result using the client's format.
+
+    :param command: Command as it would be written in a console
+    :type command: str
+    :param cwd: Current working directory path
+    :type cwd: str
+    :param timeout:
+        Maximum amount of time in seconds to wait for the command to complete
+    :type timeout: int
+    :param cmd_type: Command type as displayed in the result object
+    :type cmd_type: str
+    :param run_as: Username to use to run the command
+    :type run_as: int
+    :param battery_measurements:
+        Flag to configure when to get battery related information
+    :type battery_measurements: bool
+    :returns: Command execution result
+    :rtype: dict
+
+    .. seealso:: :func:`make_result`
+
+    """
 
     if run_as is not None:
         # add -n so sudo will not prompt for a password on the tty
@@ -172,6 +193,13 @@
     Make sure that byte strings are used only for ascii data and unicode
     strings for strings using any other encoding.
 
+    :param value: Data string that should be normalized
+    :type value: str
+    :param encoding: Encoding used to decode the given string
+    :type encoding: str
+    :returns: Data string reencoded using in ascii if possible
+    :rtype: str
+
     """
     unicode_value = value.decode(encoding)
     try:
@@ -185,10 +213,37 @@
 # TODO: it might make sense to have a result object that keeps track of
 # test pass, fail, error and serializes it's data.
 def make_result(command, retcode, stdout='', stderr='', start_time='',
-                time_delta='', cmd_type=CMD_TC_TEST, user="unknown",
+                time_delta='', cmd_type=CMD_TC_TEST, user='unknown',
                 start_battery=None, end_battery=None):
-    """
-    Make a result dictionary.
+    """Make a result data structure.
+
+    .. note::
+        Battery information will only be included if some value is passed as
+        argument.
+
+    :param command: Command that was executed
+    :type command: str
+    :param retcode: Return code
+    :type retcode: int
+    :param stdout: Standard output
+    :type stdout: str
+    :param stderr: Standard error
+    :type stderr: str
+    :param start_time: Time in which command execution started
+    :type start_time: str
+    :param time_delta: Amount of time that the command took to complete
+    :type time_delta: str
+    :param cmd_type: Command type to be displayed in the report
+    :type cmd_type: str
+    :param user: User name the executed the command
+    :type user: str
+    :param start_battery: Battery information before running the command
+    :type start_battery: dict
+    :param end_battery: Battery information after running the command
+    :type end_battery: dict
+    :returns: Result data
+    :rtype: dict
+
     """
     res = {
         'command': command,
@@ -212,6 +267,14 @@
 
 
 def get_process_children(pid):
+    """Get the children processes of a given one.
+
+    :param pid: Process ID of the parent process
+    :type pid: int
+    :returns: Process ID for the childern processes
+    :rtype: list(int)
+
+    """
     p = subprocess.Popen('ps --no-headers -o pid --ppid %d' % pid, shell=True,
                          stdout=subprocess.PIPE, stderr=subprocess.PIPE)
     stdout, _stderr = p.communicate()
@@ -220,9 +283,18 @@
 
 
 def parse_yaml_file(filename):
-    """
-    Parse yaml file and raise exception with error location
-    in case of error
+    """Parse yaml file.
+
+    :param filename: Path to the file that should be read
+    :type filename: str
+    :returns: Parsed data
+    :rtype: object
+    :raises YAMLParsingError:
+        If there's a problem while parsing the file with the information about
+        where in the file the problem was detected.
+
+    .. seealso:: :func:`parse_control_file`
+
     """
     try:
         with open(filename, 'r') as fp:
@@ -247,7 +319,16 @@
 # Validator that sets values to defaults as explained in:
 # https://github.com/Julian/jsonschema/issues/4
 class DefaultValidator(jsonschema.Validator):
+
+    """jsonschema validator that sets default values.
+
+    During the validation, if some field is missing in the data, it will be set
+    to the default value specified in the schema if defined.
+
+    """
+
     def validate_properties(self, properties, instance, schema):
+        """Set missing properties to default value."""
         (super(DefaultValidator, self)
          .validate_properties(properties, instance, schema))
         instance.update((k, v['default'])
@@ -256,8 +337,19 @@
 
 
 def parse_control_file(filename, schema):
-    """
-    Parse a control file and check against the schema
+    """Parse a control file and check against a jsonschema.
+
+    :param filename: Path to the yaml file to be parsed
+    :type filename: str
+    :param schema: jsonschema to validate data against
+    :type schema: dict
+    :returns: Parsed data
+    :rtype: object
+    :raises jsonschema.ValidationError:
+        If file contents doesn't follow the schema definitions
+
+    .. seealso:: :func:`parse_yaml_file`, :class:`DefaultValidator`
+
     """
     control_data = parse_yaml_file(filename)
     validator = DefaultValidator()
@@ -266,8 +358,17 @@
 
 
 def debug_print(data, force=False):
-    """
-    Print debugging information if CONFIG['DEBUG'] is defined and True
+    """Print debugging information according to ``CONFIG['DEBUG']`` value.
+
+    Data will be printed with the ``DEBUG:`` string prepended to it.
+
+    :param data: Data to be printed
+    :type data: object
+    :param force: Print data regardless of the configuration
+    :type force: bool
+    :returns: Whether something was printed or not
+    :rtype: bool
+
     """
     try:
         debug = CONFIG['DEBUG']
@@ -284,8 +385,11 @@
 
 
 def raise_privileges(orig_privs):
-    """
-    Raise privileges to original effective privileges.
+    """Raise privileges to original effective privileges.
+
+    :param orig_privs: Original privileges
+    :type orig_privs: dict
+
     """
     os.seteuid(orig_privs['euid'])
     os.setegid(orig_privs['egid'])
@@ -293,12 +397,18 @@
 
 
 def drop_privileges(user='nobody', group='nogroup', full=False):
-    """
-    Change user and/or group to decrease privileges.
+    """Change user and/or group to decrease privileges.
 
     Currently only drops effective privileges.
 
-    Returns a dictionary of the original values.
+    :param user: User name to change to.
+    :type user: str
+    :param group: Group name to change to.
+    :type group: str
+
+    :returns: Original prrivileges
+    :rtype: dict
+
     """
 
     orig_privs = {}
@@ -333,8 +443,13 @@
 
 
 def gather_artifacts(extras=None, excludes=None):
-    """
-    Include a set of useful artifacts (i.e. files) for the logs.
+    """Print files contents, so that it's included in the logs.
+
+    :param extras: Path to files to be included in the artifacts
+    :type extras: list(str)
+    :param excludes: Path to files to be exluded from the artifacts
+    :type excludes: list(str)
+
     """
     if extras is None:
         extras = []
@@ -366,12 +481,14 @@
 
 
 def get_media_info():
-    """
-    Get the contents of the media-info file if available.
-
-    NOTE: This is only a best-effort approach.
-
-    Returns the contents of the media-info file or None.
+    """Get the contents of the media-info file if available.
+
+    :returns:
+        The contents of the media-info file or ``'unknown'`` if not available.
+    :rtype: str
+
+    .. note:: This is only a best-effort approach.
+
     """
 
     filename = '/var/log/installer/media-info'
@@ -386,12 +503,13 @@
 
 
 def get_product_uuid():
-    """
-    Get the product_uuid of the machine under test.
-
-    NOTE: This is only a best-effort approach.
-
-    Returns the contents of the product_uuid file or None.
+    """Get the product_uuid of the machine under test.
+
+    :returns:
+        The contents of the product_uuid file or ``None`` if not available.
+
+    .. note:: This is only a best-effort approach.
+
     """
 
     filename = '/sys/class/dmi/id/product_uuid'
@@ -406,8 +524,13 @@
 
 
 def get_host_info():
-    """
-    Get host info, useful for debugging.
+    """Get host info, useful for debugging.
+
+    :returns: Host uname, media-info and product_uuid together
+    :rtype: dict
+
+    .. seealso:: :func:`get_media_info`, :func:`get_product_uuid`
+
     """
 
     retval = {}
@@ -421,10 +544,11 @@
 
 
 def get_arch():
-    """
-    The host's architecture.
-
-    Returns the human readable architecture or 'unknown'.
+    """Get the host's architecture.
+
+    :returns: The human readable architecture or ``'unknown'``
+    :rtype: str
+
     """
     arches = {
         'x86_64': 'amd64',
@@ -436,15 +560,20 @@
 
 
 def get_release():
-    """
-    The host's release name (i.e. precise, quantal, etc.)
-    """
-    return platform.dist()[2]
+    """Get the host's release name.
+
+    :returns: Release name (i.e. quantal, raring, etc.)
+    :rtype: str
+
+    """
+    return platform.linux_distribution()[2]
 
 
 def get_api_config():
-    """
-    Parse the client configuration file for API configuration data.
+    """Parse the client configuration file for API configuration data.
+
+    :returns: The value for the ``API`` field in the client configuration file.
+
     """
 
     data = {}
@@ -457,6 +586,14 @@
 
 
 def get_build_number():
+    """Get build number.
+
+    :returns: Build number as according to media-info or ``'?'`` if not found
+    :rtype: str
+
+    .. seealso:: :func:`get_host_info`
+
+    """
     host_info = get_host_info()
     pattern = re.compile('.*\(([0-9.]+)\)$')
     match = pattern.match(host_info['media-info'])
@@ -469,8 +606,19 @@
 
 
 class VCSHandler(object):
-    """
-    Base class for Version Ccontrol System support.
+
+    """Base class for Version Control System support.
+
+    :param repo: Repository location
+    :type repo: str
+    :param destination:
+        Local directory where the repository should be made available
+    :type destination: str
+    :param battery_measurements:
+        Whether battery information should be gathered when running any of the
+        VCS commands
+    :type battery_measurements: bool
+
     """
 
     def __init__(self, repo, destination='', battery_measurements=False):
@@ -479,8 +627,18 @@
         self.battery_measurements = battery_measurements
 
     def get(self, directory=REPO_DEFAULT_DIR):
-        """
-        Method to retrieve a copy of the repository
+        """Execute VCS get command.
+
+        The get command will make a copy of a given repository to the directory
+        specified as destination.
+
+        :param directory: Working directory
+        :type directory: str
+        :returns: Get command execution result
+        :rtype: dict
+
+        .. seealso:: :func:`run_cmd`
+
         """
         return run_cmd(self.get_command,
                        cwd=directory,
@@ -488,8 +646,15 @@
                        battery_measurements=self.battery_measurements)
 
     def revision(self, directory=REPO_DEFAULT_DIR):
-        """
-        Return the revision identifier.
+        """Execute revision command.
+
+        :param directory: Working directory
+        :type directory: str
+        :returns: Revision command execution result.
+        :rtype: dict
+
+        .. seealso:: :func:`run_cmd`, :meth:`get_revision`
+
         """
         return run_cmd(self.rev_command,
                        cwd=directory,
@@ -497,8 +662,18 @@
                        battery_measurements=self.battery_measurements)
 
     def get_revision(self, directory=REPO_DEFAULT_DIR):
-        """
-        get the revision information
+        """Print revision information.
+
+        This is a wrapper around :meth:`revision` to make sure that the command
+        returned a success code.
+
+        :param directory: Working directory
+        :type directory: str
+        :returns: Revision command returncode
+        :rtype: int
+
+        .. seealso:: :meth:`revision`
+
         """
         retval = None
         result = self.revision(directory=directory)
@@ -512,9 +687,8 @@
 
 
 class BzrHandler(VCSHandler):
-    """
-    bazaar handler.
-    """
+
+    """Bazaar VCS handler."""
 
     def __init__(self, repo, branch=True, options="", destination="",
                  **kwargs):
@@ -540,9 +714,8 @@
 
 
 class GitHandler(VCSHandler):
-    """
-    git handler.
-    """
+
+    """Git VCS handler."""
 
     def __init__(self, repo, options="", destination="", **kwargs):
         super(GitHandler, self).__init__(repo, destination, **kwargs)
@@ -558,7 +731,13 @@
 
 
 class DevHandler(VCSHandler):
-    """ Copy from a local directory for development. """
+
+    """Development VCS handler.
+
+    This isn't a handler for any VCS, but a helper to ease development and let
+    runlists point to a path that can be copied locally.
+
+    """
 
     def __init__(self, repo, destination=".", **kwargs):
         super(DevHandler, self).__init__(repo, destination, **kwargs)