[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 1/8] qapi/error: Repurpose QAPIError as a generic exceptio
From: |
Markus Armbruster |
Subject: |
Re: [PATCH v2 1/8] qapi/error: Repurpose QAPIError as a generic exception base class |
Date: |
Fri, 16 Apr 2021 08:04:50 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) |
John Snow <jsnow@redhat.com> writes:
> On 4/15/21 2:44 AM, Markus Armbruster wrote:
>> 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?
>>
>
> Sure. The existing QAPIError class assumes that it's an error that has a
> source location, but not all errors will. The idea is that an abstract
> error class can be used as the ancestor for all other errors in a
> type-safe way, such that:
>
> try:
> qapi.something_or_other()
> except QAPIError as err:
> err.some_sort_of_method()
>
> (1) This is a type-safe construct, and
> (2) This is sufficient to catch all errors that originate from within
> the library, regardless of their exact type.
>
> Primarily, it's a ploy to get the SourceInfo stuff out of the
> common/root exception for type safety reasons.
>
> (Later in the series, you'll see there's a few places where we try to
> fake SourceInfo stuff in order to use QAPIError, by shuffling this
> around, we allow ourselves to raise exceptions that don't need this
> hackery.)
I think you're conflating a real issue with a non-issue.
The real issue: you want to get rid of fake QAPISourceInfo. This isn't
terribly important, but small cleanups count, too. I can't see the "few
places where we try to fake" in this series, though. Is it in a later
part, or am I just blind?
The non-issue: wanting a common base class. Yes, we want one, but we
already got one: QAPIError.
I think the commit message should explain the real issue more clearly,
without confusing readers with the non-issue.
Makes sense?