John Snow <js...@redhat.com> writes: > On 4/15/21 2:44 AM, Markus Armbruster wrote: >> John Snow <js...@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?
