Merge lp:~joetalbott/utah/parser_error_handling into lp:utah

[Joe Talbott]

Add module level error handling and raise ParserError when necessary.

This branch handles schema validation errors so that the parsing process can finish.

I'm not sure if I should add my own exception class to utah.parser and have consumers of this module use that exception as an indicator of failure or use None as I'm doing in this branch. Open for discussion.

[Javier Collado (javier.collado)]

I'm not sure about the usefulness of capturing the exceptions and printing
errors without more detail about what would be the use case for it.

If the code is expected to be used as a script, then it makes sense to do this
and also to return an error code. However, if the code is expected to be used
as a library, then the calling code could capture the exceptions itself, so it
depends on the expectations of that code.

[Joe Talbott (joetalbott)]

This is library code. I don't want code consumers to have to know to catch jsonschema.ValidationError or whatever. I'm going to resubmit with a utah.parser.ParseError exception that code consumers can catch and raise that on internal errors.

[lp:~joetalbott/utah/parser_error_handling]

758. By Joe Talbott on 2012-11-27

parser - Add exception class.

[Joe Talbott (joetalbott)]

Please re-review. This branch now adds a ParserError exception class for consumers.

review: Needs Resubmitting

[Javier Collado (javier.collado)]

Since this is a small change, let me be picky and look into the documentation
strings.

Since we're using sphinx for documentation and I see you've included
information return and exception information in `UTAHParser.parse`, let's use
sphinx markup so that it shows properly in the html documentation.

To do that use (http://sphinx-doc.org/domains.html#info-field-lists):
:param <param>: to describe a parameter
:type <type>: to annotate the parameter type
:raises <exception>: to add exceptions that might be raised
:return: to describe what is being returned
:rtype: to annotate the type of the return value

You'll see that there are different options for the same markup, but if we use
the same ones it will look more consistent (actually, I've been using :throws:
instead of :raises:, but I'll fix that).

Finally, if you use the pep257 to ensure that the documentation is compliant,
then that will make me really happy.

review: Needs Fixing

[Javier Collado (javier.collado)]

I've created a merge request for the change from :throws: to :raises: and also
I've seen that what I've been using is :returns: (not :return:), so please us
that instead.

[lp:~joetalbott/utah/parser_error_handling]

759. By Joe Talbott on 2012-11-29

parser - Fix docstrings and add sphinx markup.

760. By Joe Talbott on 2012-11-29

parser - Fix some leftover docstring issues.

[Joe Talbott (joetalbott)]

Fixed docstrings.

review: Needs Resubmitting

[Javier Collado (javier.collado)]

Thanks for the update.

I'm going to merge this with some pep8 related fixes and small cosmetic changes
to the docstrings. In particular, I'm going to remove the whitespace after the
triple quotes to match the examples in pep257.

review: Approve

=== modified file 'docs/source/reference.rst'
--- docs/source/reference.rst	2012-11-26 13:32:02 +0000
+++ docs/source/reference.rst	2012-11-29 23:26:22 +0000
@@ -22,6 +22,12 @@
 .. automodule:: utah.iso
    :members:
 
+``utah.parser``
+----------------
+
+.. automodule:: utah.parser
+   :members:
+
 ``utah.preseed``
 ----------------
 

=== modified file 'utah/parser.py'
--- utah/parser.py	2012-11-21 14:59:18 +0000
+++ utah/parser.py	2012-11-29 23:26:22 +0000
@@ -1,3 +1,7 @@
+"""
+UTAH Client log parser.
+
+"""
 import argparse
 import jsonschema
 import logging
@@ -5,10 +9,15 @@
 import urllib2
 import yaml
 
+class ParserError(Exception):
+
+    """ Module level parse error exception.  """
+
+    pass
+
 class UTAHParser(object):
-    """
-    UTAH Client result parser class.
-    """
+
+    """ UTAH Client result parser class.  """
 
     COMMAND_SCHEMA = {
         'type': 'object',
@@ -71,13 +80,13 @@
     }
 
     def parse(self, logdata, as_string=False):
-        """
-        Parse utah client results.
-
-        logdata should be either a filename or a handle to a file
-        object.
-
-        Returns the parsed yaml data or None.
+        """ Parse utah client results.
+
+        :param logdata: should be either a filename or a handle to a file object.
+
+        :returns: the parsed yaml data or None.
+
+        :raises: ParserError on exceptions.
 
         """
 
@@ -89,19 +98,40 @@
             return self._parse_stream(logdata)
 
     def _parse_stream(self, stream):
-        """ Parse client output from stream. """
+        """ Parse client output from stream.
+        
+        :returns: validated decoded YAML data.
+        
+        :raises: ParseError
+        
+        """
         try:
             data = yaml.load(stream)
         except yaml.YAMLError as e:
             logging.error(e)
-            return None
-
-        jsonschema.validate(data, self.CLIENT_OUTPUT_SCHEMA)
+            raise ParserError(e.message)
+
+        # Allow empty yaml files
+        if data is None:
+            return data
+
+        try:
+            jsonschema.validate(data, self.CLIENT_OUTPUT_SCHEMA)
+        except jsonschema.ValidationError as e:
+            logging.error(e)
+            raise ParserError(e.message)
 
         return data
 
     def _parse_logfile(self, logfile):
-        """ Parse client output log. """
+        """ Parse client output log.
+        
+        :returns: validated decoded YAML data.
+        
+        :raises: ParseError
+        
+        """
+
 
         data = None
 
@@ -115,14 +145,28 @@
         return data
 
     def _parse_string(self, log_data):
-        """ Parse client data from a string. """
+        """ Parse client data from a string.
+        
+        :returns: validated decoded YAML data.
+        
+        :raises: ParseError
+        
+        """
         try:
             data = yaml.load(log_data)
+
+            # Allow empty yaml files
+            if data is None:
+                return data
         except yaml.YAMLError as e:
             logging.error(e)
-            return None
+            raise ParserError(e.message)
 
-        jsonschema.validate(data, self.CLIENT_OUTPUT_SCHEMA)
+        try:
+            jsonschema.validate(data, self.CLIENT_OUTPUT_SCHEMA)
+        except jsonschema.ValidationError as e:
+            logging.error(e)
+            raise ParserError(e.message)
 
         return data