Re: [PATCH 02/57] qapi: shush pylint up

[John Snow <jsnow@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3418 bytes --]

On Wed, Mar 5, 2025, 1:28 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > Shhhhh!
> >
> > This patch is RFC quality, I wasn't in the mood to actually solve
> > problems so much as I was in the mood to continue working on the Sphinx
> > rework. Plus, I don't think the code I am patching has hit origin/master
> > yet ...
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >  scripts/qapi/backend.py |  2 ++
> >  scripts/qapi/main.py    | 10 ++++------
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/scripts/qapi/backend.py b/scripts/qapi/backend.py
> > index 14e60aa67af..49ae6ecdd33 100644
> > --- a/scripts/qapi/backend.py
> > +++ b/scripts/qapi/backend.py
> > @@ -13,6 +13,7 @@
> >
> >
> >  class QAPIBackend(ABC):
> > +    # pylint: disable=too-few-public-methods
> >
> >      @abstractmethod
> >      def generate(self,
> > @@ -36,6 +37,7 @@ def generate(self,
> >
> >
> >  class QAPICBackend(QAPIBackend):
> > +    # pylint: disable=too-few-public-methods
> >
> >      def generate(self,
> >                   schema: QAPISchema,
>
> I didn't bother to ask for these in my review.  Do they get us to the
> point where we can enable automatic pylint?
>

Yes.


> > diff --git a/scripts/qapi/main.py b/scripts/qapi/main.py
> > index ff42b43cd68..01155373bd0 100644
> > --- a/scripts/qapi/main.py
> > +++ b/scripts/qapi/main.py
> > @@ -31,15 +31,14 @@ def create_backend(path: str) -> QAPIBackend:
> >
> >      module_path, dot, class_name = path.rpartition('.')
> >      if not dot:
> > -        print(f"argument of -B must be of the form MODULE.CLASS",
> > +        print("argument of -B must be of the form MODULE.CLASS",
>
> This one's already in "[PULL v2 0/5] QAPI patches patches for
> 2025-02-26".  No worries.
>

Got it. I'll repull your tags.


> >                file=sys.stderr)
> >          sys.exit(1)
> >
> >      try:
> >          mod = import_module(module_path)
> >      except Exception as ex:
> > -        print(f"unable to import '{module_path}': {ex}",
> file=sys.stderr)
> > -        sys.exit(1)
> > +        raise QAPIError(f"unable to import '{module_path}': {ex}") from
> ex
> >
> >      try:
> >          klass = getattr(mod, class_name)
> > @@ -51,9 +50,8 @@ def create_backend(path: str) -> QAPIBackend:
> >      try:
> >          backend = klass()
> >      except Exception as ex:
> > -        print(f"backend '{path}' cannot be instantiated: {ex}",
> > -              file=sys.stderr)
> > -        sys.exit(1)
> > +        raise QAPIError(
> > +            f"backend '{path}' cannot be instantiated: {ex}") from ex
> >
> >      if not isinstance(backend, QAPIBackend):
> >          print(f"backend '{path}' must be an instance of QAPIBackend",
>
> Missed in my review: the caller catches QAPIError, and returns non-zero
> exit code on catch.  The caller's caller passes the exit code to
> sys.exit().  Leaving the sys.exit() to the callers is cleaner.
>
> However, you convert only two out of five error paths from sys.exit() to
> raise.  All or nothing, please.
>

"RFC Quality" ;)

You got it, I'll be consistent in approach here. My initial goal was only
to get the linters clean for this series.


> Maybe split the patch into a "# pylint:" and a "raise QAPIError" part?
>

Sure.

>

[-- Attachment #2: Type: text/html, Size: 5640 bytes --]