Re: [PATCH 1/1] sphinx/qapidoc: Tidy up pylint warning raise-missing-from

[Peter Maydell]

On Fri, 3 Nov 2023 at 16:02, Markus Armbruster <armbru@redhat.com> wrote:
>
> Peter Maydell <peter.maydell@linaro.org> writes:
>
> > On Fri, 3 Nov 2023 at 03:08, John Snow <jsnow@redhat.com> wrote:
> >>
> >> On Wed, Oct 25, 2023 at 6:10 AM Markus Armbruster <armbru@redhat.com> 
> >> wrote:
> >> >
> >> > Pylint advises:
> >> >
> >> >     docs/sphinx/qapidoc.py:518:12: W0707: Consider explicitly re-raising 
> >> > using 'raise ExtensionError(str(err)) from err' (raise-missing-from)
> >> >
> >> > From its manual:
> >> >
> >> >     Python's exception chaining shows the traceback of the current
> >> >     exception, but also of the original exception.  When you raise a
> >> >     new exception after another exception was caught it's likely that
> >> >     the second exception is a friendly re-wrapping of the first
> >> >     exception.  In such cases `raise from` provides a better link
> >> >     between the two tracebacks in the final error.
> >> >
> >> > Makes sense, so do it.
> >> >
> >> > Signed-off-by: Markus Armbruster <armbru@redhat.com>
> >>
> >> In this case it probably doesn't make a difference because Sphinx has
> >> its own formatting for displaying the errors, but it's good hygiene.
> >>
> >> Reviewed-by: John Snow <jsnow@redhat.com>
> >
> > Has somebody checked that the error Sphinx shows to the user
> > is still the friendly one? The only reason to raise
> > this error is so that Sphinx will catch it and display
> > the friendly string, so anything about tracebacks is a red
> > herring -- if the traceback is shown to the user then we got
> > something wrong.
>
> The exception type doesn't change, only the backtrace stored within the
> exception.  Can't see how its catching could be affected.
>
> To be sure, stick a '} at the beginning of qapi-schema.json and run
> sphinx-build.  Result:
>
>     Extension error:
>     /work/armbru/qemu/docs/../qapi/qapi-schema.json:1:1: expected '{', '[', 
> string, or boolean
>
> Satisfied?

Yep, looks good, thanks. (That is, this change is purely
to shut up pylint, and it doesn't have any bad side effects.)

-- PMM