Re: [PATCH v2 1/8] qapi/error: Repurpose QAPIError as a generic exception base class

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> Rename QAPIError to QAPISourceError, and then create a new QAPIError
> class that serves as the basis for all of our other custom exceptions.

Isn't the existing QAPIError such a base class already?  Peeking
ahead...  aha, your new base class is abstract.  Can you explain why we
that's useful?

> Add docstrings to explain the intended function of each error class.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  docs/sphinx/qapidoc.py |  3 ++-
>  scripts/qapi/error.py  | 11 +++++++++--
>  scripts/qapi/schema.py |  5 +++--
>  3 files changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
> index b7b86b5dff..458b1f477e 100644
> --- a/docs/sphinx/qapidoc.py
> +++ b/docs/sphinx/qapidoc.py
> @@ -34,7 +34,8 @@
>  from sphinx.util.nodes import nested_parse_with_titles
>  import sphinx
>  from qapi.gen import QAPISchemaVisitor
> -from qapi.schema import QAPIError, QAPISemError, QAPISchema
> +from qapi.error import QAPIError, QAPISemError
> +from qapi.schema import QAPISchema
>  
>  
>  # Sphinx up to 1.6 uses AutodocReporter; 1.7 and later
> diff --git a/scripts/qapi/error.py b/scripts/qapi/error.py
> index ae60d9e2fe..126dda7c9b 100644
> --- a/scripts/qapi/error.py
> +++ b/scripts/qapi/error.py
> @@ -13,6 +13,11 @@
>  
>  
>  class QAPIError(Exception):
> +    """Base class for all exceptions from the QAPI package."""
> +
> +
> +class QAPISourceError(QAPIError):
> +    """Error class for all exceptions identifying a source location."""
>      def __init__(self, info, col, msg):
>          Exception.__init__(self)
>          self.info = info
> @@ -27,7 +32,8 @@ def __str__(self):
>          return loc + ': ' + self.msg
>  
>  
> -class QAPIParseError(QAPIError):
> +class QAPIParseError(QAPISourceError):
> +    """Error class for all QAPI schema parsing errors."""
>      def __init__(self, parser, msg):
>          col = 1
>          for ch in parser.src[parser.line_pos:parser.pos]:
> @@ -38,6 +44,7 @@ def __init__(self, parser, msg):
>          super().__init__(parser.info, col, msg)
>  
>  
> -class QAPISemError(QAPIError):
> +class QAPISemError(QAPISourceError):
> +    """Error class for semantic QAPI errors."""
>      def __init__(self, info, msg):
>          super().__init__(info, None, msg)
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 01cdd753cd..1849c3e45f 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -20,7 +20,7 @@
>  from typing import Optional
>  
>  from .common import POINTER_SUFFIX, c_name
> -from .error import QAPIError, QAPISemError
> +from .error import QAPISemError, QAPISourceError
>  from .expr import check_exprs
>  from .parser import QAPISchemaParser
>  
> @@ -878,7 +878,8 @@ def _def_entity(self, ent):
>          other_ent = self._entity_dict.get(ent.name)
>          if other_ent:
>              if other_ent.info:
> -                where = QAPIError(other_ent.info, None, "previous 
> definition")
> +                where = QAPISourceError(other_ent.info, None,
> +                                        "previous definition")
>                  raise QAPISemError(
>                      ent.info,
>                      "'%s' is already defined\n%s" % (ent.name, where))