diff mbox series

event: add bb.event.ParseError

Message ID 20230410091900.2897894-1-mingli.yu@eng.windriver.com
State Accepted, archived
Commit 316524ab59a5e738c25e062923ee5717d88ae5c7
Headers show
Series event: add bb.event.ParseError | expand

Commit Message

mingli.yu@eng.windriver.com April 10, 2023, 9:19 a.m. UTC 
From: Mingli Yu <mingli.yu@windriver.com>

Add bb.event.ParseError to let error-report can catch this kind of error.

Signed-off-by: Mingli Yu <mingli.yu@windriver.com>
---
 lib/bb/cooker.py | 6 ++++--
 lib/bb/event.py  | 8 ++++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

Comments

Frédéric Martinsons April 10, 2023, 9:43 a.m. UTC | #1 
On Mon, 10 Apr 2023 at 11:19, Yu, Mingli <mingli.yu@eng.windriver.com>
wrote:

> From: Mingli Yu <mingli.yu@windriver.com>
>
> Add bb.event.ParseError to let error-report can catch this kind of error.
>
> Signed-off-by: Mingli Yu <mingli.yu@windriver.com>
> ---
>  lib/bb/cooker.py | 6 ++++--
>  lib/bb/event.py  | 8 ++++++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
> index 1797a1d4..206f8ffb 100644
> --- a/lib/bb/cooker.py
> +++ b/lib/bb/cooker.py
> @@ -2223,7 +2223,7 @@ class CookerParser(object):
>
>              self.results = itertools.chain(self.results,
> self.parse_generator())
>
> -    def shutdown(self, clean=True):
> +    def shutdown(self, clean=True, eventmsg=""):
>          if not self.toparse:
>              return
>          if self.haveshutdown:
> @@ -2238,6 +2238,8 @@ class CookerParser(object):
>
>              bb.event.fire(event, self.cfgdata)
>          else:
> +            if eventmsg:
> +                bb.event.fire(bb.event.ParseError(eventmsg), self.cfgdata)
>              bb.error("Parsing halted due to errors, see error messages
> above")
>
>          # Cleanup the queue before call process.join(), otherwise there
> might be
> @@ -2355,7 +2357,7 @@ class CookerParser(object):
>          except bb.parse.ParseError as exc:
>              self.error += 1
>              logger.error(str(exc))
> -            self.shutdown(clean=False)
> +            self.shutdown(clean=False, eventmsg=str(exc))
>              return False
>          except bb.data_smart.ExpansionError as exc:
>              self.error += 1
> diff --git a/lib/bb/event.py b/lib/bb/event.py
> index 37cc630c..72393829 100644
> --- a/lib/bb/event.py
> +++ b/lib/bb/event.py
> @@ -856,3 +856,11 @@ class FindSigInfoResult(Event):
>      def __init__(self, result):
>          Event.__init__(self)
>          self.result = result
> +
> +class ParseError(Event):
> +    """
> +    Event to indicate parse failed
> +    """
> +    def __init__(self, msg):
> +        Event.__init__(self)
> +        self._msg = msg
> --
> 2.25.1
>
>
Can't we just use super ?
super().__init__() instead of Event.__init__(self)
I didn't find a reference but I think it is a more pythonic way of calling
parent class.
Richard Purdie April 11, 2023, 4:50 p.m. UTC | #2 
On Mon, 2023-04-10 at 17:19 +0800, Yu, Mingli wrote:
> From: Mingli Yu <mingli.yu@windriver.com>
> 
> Add bb.event.ParseError to let error-report can catch this kind of error.
> 
> Signed-off-by: Mingli Yu <mingli.yu@windriver.com>
> ---
>  lib/bb/cooker.py | 6 ++++--
>  lib/bb/event.py  | 8 ++++++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
> index 1797a1d4..206f8ffb 100644
> --- a/lib/bb/cooker.py
> +++ b/lib/bb/cooker.py
> @@ -2223,7 +2223,7 @@ class CookerParser(object):
>  
>              self.results = itertools.chain(self.results, self.parse_generator())
>  
> -    def shutdown(self, clean=True):
> +    def shutdown(self, clean=True, eventmsg=""):
>          if not self.toparse:
>              return
>          if self.haveshutdown:
> @@ -2238,6 +2238,8 @@ class CookerParser(object):
>  
>              bb.event.fire(event, self.cfgdata)
>          else:
> +            if eventmsg:
> +                bb.event.fire(bb.event.ParseError(eventmsg), self.cfgdata)
>              bb.error("Parsing halted due to errors, see error messages above")

Shouldn't this always fire the event even if there is no specific error
message?

Cheers,

Richard
Yu, Mingli April 12, 2023, 2:42 a.m. UTC | #3 
Hi Richard,

On 4/12/23 00:50, Richard Purdie wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> On Mon, 2023-04-10 at 17:19 +0800, Yu, Mingli wrote:
>> From: Mingli Yu <mingli.yu@windriver.com>
>>
>> Add bb.event.ParseError to let error-report can catch this kind of error.
>>
>> Signed-off-by: Mingli Yu <mingli.yu@windriver.com>
>> ---
>>   lib/bb/cooker.py | 6 ++++--
>>   lib/bb/event.py  | 8 ++++++++
>>   2 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
>> index 1797a1d4..206f8ffb 100644
>> --- a/lib/bb/cooker.py
>> +++ b/lib/bb/cooker.py
>> @@ -2223,7 +2223,7 @@ class CookerParser(object):
>>
>>               self.results = itertools.chain(self.results, self.parse_generator())
>>
>> -    def shutdown(self, clean=True):
>> +    def shutdown(self, clean=True, eventmsg=""):
>>           if not self.toparse:
>>               return
>>           if self.haveshutdown:
>> @@ -2238,6 +2238,8 @@ class CookerParser(object):
>>
>>               bb.event.fire(event, self.cfgdata)
>>           else:
>> +            if eventmsg:
>> +                bb.event.fire(bb.event.ParseError(eventmsg), self.cfgdata)
>>               bb.error("Parsing halted due to errors, see error messages above")
> 
> Shouldn't this always fire the event even if there is no specific error
> message?

There are many kinds of exception here and each exception will call 
shudown function, but we only catch bb.parse.ParseError exception and 
pass eventmsg to shudown function when it is bb.parse.ParseError. So 
checking eventmsg in shutdown function should be okay.

except bb.parse.ParseError as exc:
             self.error += 1
             logger.error(str(exc))
             self.shutdown(clean=False, eventmsg=str(exc))
             return False

Thanks,

> 
> Cheers,
> 
> Richard
>
Richard Purdie April 12, 2023, 6:58 a.m. UTC | #4 
On Wed, 2023-04-12 at 10:42 +0800, Yu, Mingli wrote:
> Hi Richard,
> 
> On 4/12/23 00:50, Richard Purdie wrote:
> > CAUTION: This email comes from a non Wind River email account!
> > Do not click links or open attachments unless you recognize the sender and know the content is safe.
> > 
> > On Mon, 2023-04-10 at 17:19 +0800, Yu, Mingli wrote:
> > > From: Mingli Yu <mingli.yu@windriver.com>
> > > 
> > > Add bb.event.ParseError to let error-report can catch this kind of error.
> > > 
> > > Signed-off-by: Mingli Yu <mingli.yu@windriver.com>
> > > ---
> > >   lib/bb/cooker.py | 6 ++++--
> > >   lib/bb/event.py  | 8 ++++++++
> > >   2 files changed, 12 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
> > > index 1797a1d4..206f8ffb 100644
> > > --- a/lib/bb/cooker.py
> > > +++ b/lib/bb/cooker.py
> > > @@ -2223,7 +2223,7 @@ class CookerParser(object):
> > > 
> > >               self.results = itertools.chain(self.results, self.parse_generator())
> > > 
> > > -    def shutdown(self, clean=True):
> > > +    def shutdown(self, clean=True, eventmsg=""):
> > >           if not self.toparse:
> > >               return
> > >           if self.haveshutdown:
> > > @@ -2238,6 +2238,8 @@ class CookerParser(object):
> > > 
> > >               bb.event.fire(event, self.cfgdata)
> > >           else:
> > > +            if eventmsg:
> > > +                bb.event.fire(bb.event.ParseError(eventmsg), self.cfgdata)
> > >               bb.error("Parsing halted due to errors, see error messages above")
> > 
> > Shouldn't this always fire the event even if there is no specific error
> > message?
> 
> There are many kinds of exception here and each exception will call 
> shudown function, but we only catch bb.parse.ParseError exception and 
> pass eventmsg to shudown function when it is bb.parse.ParseError. So 
> checking eventmsg in shutdown function should be okay.
> 
> except bb.parse.ParseError as exc:
>              self.error += 1
>              logger.error(str(exc))
>              self.shutdown(clean=False, eventmsg=str(exc))
>              return False

I don't mean to call it totally unconditionally, only in the same cases
as we show the bb.error()

Cheers,

Richard
diff mbox series

Patch

diff --git a/lib/bb/cooker.py b/lib/bb/cooker.py
index 1797a1d4..206f8ffb 100644
--- a/lib/bb/cooker.py
+++ b/lib/bb/cooker.py
@@ -2223,7 +2223,7 @@  class CookerParser(object):
 
             self.results = itertools.chain(self.results, self.parse_generator())
 
-    def shutdown(self, clean=True):
+    def shutdown(self, clean=True, eventmsg=""):
         if not self.toparse:
             return
         if self.haveshutdown:
@@ -2238,6 +2238,8 @@  class CookerParser(object):
 
             bb.event.fire(event, self.cfgdata)
         else:
+            if eventmsg:
+                bb.event.fire(bb.event.ParseError(eventmsg), self.cfgdata)
             bb.error("Parsing halted due to errors, see error messages above")
 
         # Cleanup the queue before call process.join(), otherwise there might be
@@ -2355,7 +2357,7 @@  class CookerParser(object):
         except bb.parse.ParseError as exc:
             self.error += 1
             logger.error(str(exc))
-            self.shutdown(clean=False)
+            self.shutdown(clean=False, eventmsg=str(exc))
             return False
         except bb.data_smart.ExpansionError as exc:
             self.error += 1
diff --git a/lib/bb/event.py b/lib/bb/event.py
index 37cc630c..72393829 100644
--- a/lib/bb/event.py
+++ b/lib/bb/event.py
@@ -856,3 +856,11 @@  class FindSigInfoResult(Event):
     def __init__(self, result):
         Event.__init__(self)
         self.result = result
+
+class ParseError(Event):
+    """
+    Event to indicate parse failed
+    """
+    def __init__(self, msg):
+        Event.__init__(self)
+        self._msg = msg