Re: [PATCH v2 05/12] simpletrace: Changed Analyzer class to become context-manager

[Stefan Hajnoczi <stefanha@redhat.com>]

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

On Tue, May 02, 2023 at 11:23:32AM +0200, Mads Ynddal wrote:
> From: Mads Ynddal <m.ynddal@samsung.com>
> 
> Instead of explicitly calling `begin` and `end`, we can change the class
> to use the context-manager paradigm. This is mostly a styling choice,
> used in modern Python code. But it also allows for more advanced analyzers
> to handle exceptions gracefully in the `__exit__` method (not
> demonstrated here).
> 
> Signed-off-by: Mads Ynddal <m.ynddal@samsung.com>
> ---
>  scripts/simpletrace.py | 40 +++++++++++++++++++++++-----------------
>  1 file changed, 23 insertions(+), 17 deletions(-)
> 
> diff --git a/scripts/simpletrace.py b/scripts/simpletrace.py
> index 7444a6e090..10ca093046 100755
> --- a/scripts/simpletrace.py
> +++ b/scripts/simpletrace.py
> @@ -121,12 +121,12 @@ def read_trace_records(event_mapping, event_id_to_name, fobj):
>  
>              yield rec
>  
> -class Analyzer(object):
> +class Analyzer:
>      """A trace file analyzer which processes trace records.
>  
> -    An analyzer can be passed to run() or process().  The begin() method is
> -    invoked, then each trace record is processed, and finally the end() method
> -    is invoked.
> +    An analyzer can be passed to run() or process().  The __enter__() method is
> +    invoked when opening the analyzer using the `with` statement, then each trace
> +    record is processed, and finally the __exit__() method is invoked.

Bearing in mind compatibility with existing simpletrace analysis
scripts, how about the following default method implementations?

  def __enter__(self):
      self.begin()

  def __exit__(self, exc_type, exc_val, exc_tb):
      if exc_type is None:
          self.end()
      return False

Now simpletrace.py can switch to using the context manager and new
scripts can implement __enter__()/__exit__(), while old scripts continue
to work.

>  
>      If a method matching a trace event name exists, it is invoked to process
>      that trace record.  Otherwise the catchall() method is invoked.
> @@ -152,19 +152,25 @@ def runstate_set(self, timestamp, pid, new_state):
>            ...
>      """
>  
> -    def begin(self):
> +    def __enter__(self):
>          """Called at the start of the trace."""
> -        pass
> +        return self
>  
>      def catchall(self, event, rec):
>          """Called if no specific method for processing a trace event has been found."""
>          pass
>  
> -    def end(self):
> +    def __exit__(self, _type, value, traceback):
>          """Called at the end of the trace."""
>          pass
>  
> -def process(events, log, analyzer, read_header=True):
> +    def __call__(self):
> +        """Fix for legacy use without context manager.
> +        We call the provided object in `process` regardless of it being the object-type or instance.
> +        With this function, it will work in both cases."""
> +        return self
> +
> +def process(events, log, analyzer_class, read_header=True):

Please don't change the function signature since this is a public method
and we should avoid breaking existing callers when possible.

Instead of:

  with analyzer_class() as analyzer:

we can use:

  with analyzer:
      ...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]