diff mbox series

[toaster,PATCHv3,3/3] toaster: Update eventreplay functionality for new eventlog file structure

Message ID 20231206224305.34686-4-marlon.rodriguez-garcia@savoirfairelinux.com
State New
Headers show
Series toaster: feat to import eventlogs] | expand

Commit Message

Marlon Rodriguez Garcia Dec. 6, 2023, 10:43 p.m. UTC
Added class EventPlayer to list of libraries under bitbake/bb/ui/
Update file read functionality to match new eventlog format

Exclude listing of files that don't contain the allvariables definitions used to replay builds
This part of the feature should be revisited. Over a long period of time, the BB_DEFAULT_EVENTLOG
will exponentially increase the size of the log file and cause bottlenecks when importing.

Signed-off-by: Marlon Rodriguez Garcia <marlon.rodriguez-garcia@savoirfairelinux.com>
---
 lib/bb/ui/eventreplay.py        |  85 +++++++++++++++++++++
 lib/toaster/toastergui/views.py | 131 ++++++++++++--------------------
 2 files changed, 132 insertions(+), 84 deletions(-)
 create mode 100644 lib/bb/ui/eventreplay.py

Comments

Richard Purdie Dec. 6, 2023, 11:02 p.m. UTC | #1
On Wed, 2023-12-06 at 17:43 -0500, Marlon Rodriguez Garcia wrote:
> Added class EventPlayer to list of libraries under bitbake/bb/ui/
> Update file read functionality to match new eventlog format
> 
> Exclude listing of files that don't contain the allvariables definitions used to replay builds
> This part of the feature should be revisited. Over a long period of time, the BB_DEFAULT_EVENTLOG
> will exponentially increase the size of the log file and cause bottlenecks when importing.
> 
> Signed-off-by: Marlon Rodriguez Garcia <marlon.rodriguez-garcia@savoirfairelinux.com>
> ---
>  lib/bb/ui/eventreplay.py        |  85 +++++++++++++++++++++
>  lib/toaster/toastergui/views.py | 131 ++++++++++++--------------------
>  2 files changed, 132 insertions(+), 84 deletions(-)
>  create mode 100644 lib/bb/ui/eventreplay.py

Our patches overlapped!

I've merged a patch to fix toaster-eventreplay and it is slightly
different to the code below. The major tweaks are handling a second
'allvariables' entry and adding a eventfile.seek(0) to reset the stream
before calling the player.

We should probably have a standalone patch moving that code from
toaster-eventreplay to lib/bb/ui/eventreplay.py and making it use it,
when can then be followed by the patch below to have views.py use it as
well?

Cheers,

Richard
Marlon Rodriguez Garcia Dec. 6, 2023, 11:32 p.m. UTC | #2
I think this could be merge as it is now, and another patch will be added to update the Views and the eventreplay lib, can you share the patch with the new changes ?

----- Original Message -----
From: "richard purdie" <richard.purdie@linuxfoundation.org>
To: "Marlon Rodriguez Garcia" <marlon.rodriguez-garcia@savoirfairelinux.com>, bitbake-devel@lists.openembedded.org, toaster@lists.yoctoproject.org
Sent: Wednesday, December 6, 2023 6:02:09 PM
Subject: Re: [toaster][PATCHv3 3/3] toaster: Update eventreplay functionality for new eventlog file structure

On Wed, 2023-12-06 at 17:43 -0500, Marlon Rodriguez Garcia wrote:
> Added class EventPlayer to list of libraries under bitbake/bb/ui/
> Update file read functionality to match new eventlog format
> 
> Exclude listing of files that don't contain the allvariables definitions used to replay builds
> This part of the feature should be revisited. Over a long period of time, the BB_DEFAULT_EVENTLOG
> will exponentially increase the size of the log file and cause bottlenecks when importing.
> 
> Signed-off-by: Marlon Rodriguez Garcia <marlon.rodriguez-garcia@savoirfairelinux.com>
> ---
>  lib/bb/ui/eventreplay.py        |  85 +++++++++++++++++++++
>  lib/toaster/toastergui/views.py | 131 ++++++++++++--------------------
>  2 files changed, 132 insertions(+), 84 deletions(-)
>  create mode 100644 lib/bb/ui/eventreplay.py

Our patches overlapped!

I've merged a patch to fix toaster-eventreplay and it is slightly
different to the code below. The major tweaks are handling a second
'allvariables' entry and adding a eventfile.seek(0) to reset the stream
before calling the player.

We should probably have a standalone patch moving that code from
toaster-eventreplay to lib/bb/ui/eventreplay.py and making it use it,
when can then be followed by the patch below to have views.py use it as
well?

Cheers,

Richard



-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#6060): https://lists.yoctoproject.org/g/toaster/message/6060
Mute This Topic: https://lists.yoctoproject.org/mt/103023929/7896800
Group Owner: toaster+owner@lists.yoctoproject.org
Unsubscribe: https://lists.yoctoproject.org/g/toaster/unsub [marlon.rodriguez-garcia@savoirfairelinux.com]
-=-=-=-=-=-=-=-=-=-=-=-
Richard Purdie Dec. 6, 2023, 11:41 p.m. UTC | #3
On Wed, 2023-12-06 at 18:32 -0500, Marlon Rodriguez Garcia wrote:
> I think this could be merge as it is now, and another patch will be
> added to update the Views and the eventreplay lib, can you share the
> patch with the new changes ?

It is sub optimal that the series adds the code to views.py in 1/3 and
then moves it again in 3/3 so I'm leaning to reworking the patches to
clean things up.

The patches still need work since they cause two existing tests to
regress too (as far as I can tell anyway).

The changes to eventreplay merged here:

https://git.yoctoproject.org/poky/commit/?id=3ee5c86da3773deb091e24b98ad592c5d19274fb

Cheers,

Richard 

> 
> ----- Original Message -----
> From: "richard purdie" <richard.purdie@linuxfoundation.org>
> To: "Marlon Rodriguez Garcia" <marlon.rodriguez-garcia@savoirfairelinux.com>, bitbake-devel@lists.openembedded.org, toaster@lists.yoctoproject.org
> Sent: Wednesday, December 6, 2023 6:02:09 PM
> Subject: Re: [toaster][PATCHv3 3/3] toaster: Update eventreplay functionality for new eventlog file structure
> 
> On Wed, 2023-12-06 at 17:43 -0500, Marlon Rodriguez Garcia wrote:
> > Added class EventPlayer to list of libraries under bitbake/bb/ui/
> > Update file read functionality to match new eventlog format
> > 
> > Exclude listing of files that don't contain the allvariables definitions used to replay builds
> > This part of the feature should be revisited. Over a long period of time, the BB_DEFAULT_EVENTLOG
> > will exponentially increase the size of the log file and cause bottlenecks when importing.
> > 
> > Signed-off-by: Marlon Rodriguez Garcia <marlon.rodriguez-garcia@savoirfairelinux.com>
> > ---
> >  lib/bb/ui/eventreplay.py        |  85 +++++++++++++++++++++
> >  lib/toaster/toastergui/views.py | 131 ++++++++++++--------------------
> >  2 files changed, 132 insertions(+), 84 deletions(-)
> >  create mode 100644 lib/bb/ui/eventreplay.py
> 
> Our patches overlapped!
> 
> I've merged a patch to fix toaster-eventreplay and it is slightly
> different to the code below. The major tweaks are handling a second
> 'allvariables' entry and adding a eventfile.seek(0) to reset the stream
> before calling the player.
> 
> We should probably have a standalone patch moving that code from
> toaster-eventreplay to lib/bb/ui/eventreplay.py and making it use it,
> when can then be followed by the patch below to have views.py use it as
> well?
> 
> Cheers,
> 
> Richard
> 
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#6060): https://lists.yoctoproject.org/g/toaster/message/6060
> Mute This Topic: https://lists.yoctoproject.org/mt/103023929/7896800
> Group Owner: toaster+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/toaster/unsub [marlon.rodriguez-garcia@savoirfairelinux.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
diff mbox series

Patch

diff --git a/lib/bb/ui/eventreplay.py b/lib/bb/ui/eventreplay.py
new file mode 100644
index 00000000..b5999a1b
--- /dev/null
+++ b/lib/bb/ui/eventreplay.py
@@ -0,0 +1,85 @@ 
+#!/usr/bin/env python3
+#
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# This file re-uses code spread throughout other Bitbake source files.
+# As such, all other copyrights belong to their own right holders.
+#
+
+
+import os
+import sys
+import json
+import pickle
+import codecs
+
+
+class EventPlayer:
+    """Emulate a connection to a bitbake server."""
+
+    def __init__(self, eventfile, variables):
+        self.eventfile = eventfile
+        self.variables = variables
+        self.eventmask = []
+
+    def waitEvent(self, _timeout):
+        """Read event from the file."""
+        line = self.eventfile.readline().strip()
+        if not line:
+            return
+        try:
+            decodedline = json.loads(line)
+            if 'allvariables' in decodedline:
+                return
+            if not 'vars' in decodedline:
+                raise ValueError
+            event_str = decodedline['vars'].encode('utf-8')
+            event = pickle.loads(codecs.decode(event_str, 'base64'))
+            event_name = "%s.%s" % (event.__module__, event.__class__.__name__)
+            if event_name not in self.eventmask:
+                return
+            return event
+        except ValueError as err:
+            print("Failed loading ", line)
+            raise err
+
+    def runCommand(self, command_line):
+        """Emulate running a command on the server."""
+        name = command_line[0]
+
+        if name == "getVariable":
+            var_name = command_line[1]
+            variable = self.variables.get(var_name)
+            if variable:
+                return variable['v'], None
+            return None, "Missing variable %s" % var_name
+
+        elif name == "getAllKeysWithFlags":
+            dump = {}
+            flaglist = command_line[1]
+            for key, val in self.variables.items():
+                try:
+                    if not key.startswith("__"):
+                        dump[key] = {
+                            'v': val['v'],
+                            'history' : val['history'],
+                        }
+                        for flag in flaglist:
+                            dump[key][flag] = val[flag]
+                except Exception as err:
+                    print(err)
+            return (dump, None)
+
+        elif name == 'setEventMask':
+            self.eventmask = command_line[-1]
+            return True, None
+
+        else:
+            raise Exception("Command %s not implemented" % command_line[0])
+
+    def getEventHandle(self):
+        """
+        This method is called by toasterui.
+        The return value is passed to self.runCommand but not used there.
+        """
+        pass
\ No newline at end of file
diff --git a/lib/toaster/toastergui/views.py b/lib/toaster/toastergui/views.py
index 2e34c8a2..cac72d36 100644
--- a/lib/toaster/toastergui/views.py
+++ b/lib/toaster/toastergui/views.py
@@ -8,12 +8,12 @@ 
 
 import ast
 import re
-import pickle
-import codecs
 import subprocess
+import sys
 
 import bb.cooker
 from bb.ui import toasterui
+from bb.ui import eventreplay
 
 from django.db.models import F, Q, Sum
 from django.db import IntegrityError
@@ -1957,71 +1957,6 @@  if True:
         except (ObjectDoesNotExist, IOError):
             return toaster_render(request, "unavailable_artifact.html")
 
-class EventPlayer:
-    """Emulate a connection to a bitbake server."""
-
-    def __init__(self, eventfile, variables):
-        self.eventfile = eventfile
-        self.variables = variables
-        self.eventmask = []
-
-    def waitEvent(self, _timeout):
-        """Read event from the file."""
-        line = self.eventfile.readline().strip()
-        if not line:
-            return
-        try:
-            event_str = json.loads(line)['vars'].encode('utf-8')
-            event = pickle.loads(codecs.decode(event_str, 'base64'))
-            event_name = "%s.%s" % (event.__module__, event.__class__.__name__)
-            if event_name not in self.eventmask:
-                return
-            return event
-        except ValueError as err:
-            print("Failed loading ", line)
-            raise err
-
-    def runCommand(self, command_line):
-        """Emulate running a command on the server."""
-        name = command_line[0]
-
-        if name == "getVariable":
-            var_name = command_line[1]
-            variable = self.variables.get(var_name)
-            if variable:
-                return variable['v'], None
-            return None, "Missing variable %s" % var_name
-
-        elif name == "getAllKeysWithFlags":
-            dump = {}
-            flaglist = command_line[1]
-            for key, val in self.variables.items():
-                try:
-                    if not key.startswith("__"):
-                        dump[key] = {
-                            'v': val['v'],
-                            'history' : val['history'],
-                        }
-                        for flag in flaglist:
-                            dump[key][flag] = val[flag]
-                except Exception as err:
-                    print(err)
-            return (dump, None)
-
-        elif name == 'setEventMask':
-            self.eventmask = command_line[-1]
-            return True, None
-
-        else:
-            raise Exception("Command %s not implemented" % command_line[0])
-
-    def getEventHandle(self):
-        """
-        This method is called by toasterui.
-        The return value is passed to self.runCommand but not used there.
-        """
-        pass
-
 
 class CommandLineBuilds(TemplateView):
     model = EventLogsImports
@@ -2038,7 +1973,14 @@  class CommandLineBuilds(TemplateView):
             files_list = []
 
             # Filter files that end with ".json"
-            event_files = [file for file in files if file.endswith(".json")]
+            event_files = []
+            for file in files:
+                if file.endswith(".json"):
+                    # because BB_DEFAULT_EVENTLOG is a directory, we need to check if the file is a valid eventlog
+                    with open("{}/{}".format(logs_dir, file)) as efile:
+                        content = efile.read()
+                        if 'allvariables' in content:
+                            event_files.append(file)
 
             #build dict for template using db data
             for event_file in event_files:
@@ -2077,7 +2019,6 @@  class CommandLineBuilds(TemplateView):
         imported_files = EventLogsImports.objects.all()
         try:
             if all_files == 'true':
-
                 # use of session variable to deactivate icon for builds in progress
                 request.session['all_builds'] = True
                 request.session.modified = True
@@ -2090,10 +2031,17 @@  class CommandLineBuilds(TemplateView):
                     else:
                         with open("{}/{}".format(logs_dir, file.get('name'))) as eventfile:
                             # load variables from the first line
-                            variables = json.loads(eventfile.readline().strip())['allvariables']
-
+                            variables = None
+                            while line := eventfile.readline().strip():
+                                try:
+                                    variables = json.loads(line)['allvariables']
+                                    break
+                                except (KeyError, json.JSONDecodeError):
+                                    continue
+                            if not variables:
+                                raise Exception("File content missing  build variables")
                             params = namedtuple('ConfigParams', ['observe_only'])(True)
-                            player = EventPlayer(eventfile, variables)
+                            player = eventreplay.EventPlayer(eventfile, variables)
 
                             toasterui.main(player, player, params)
                         event_log_import = EventLogsImports.objects.create(name=file.get('name'), imported=True)
@@ -2104,20 +2052,27 @@  class CommandLineBuilds(TemplateView):
                     file = self.request.FILES['eventlog_file']
                 else:
                     file = request.POST.get('file')
-
-                # use of session variable to deactivate icon for build in progress 
-                request.session['file'] = file
-                request.session['all_builds'] = False
-                request.session.modified = True
-                request.session.save()
+                    # use of session variable to deactivate icon for build in progress 
+                    request.session['file'] = file
+                    request.session['all_builds'] = False
+                    request.session.modified = True
+                    request.session.save()
 
                 if imported_files.filter(name=file).exists():
                     imported_files.filter(name=file)[0].imported = True
                 else: 
                     if isinstance(file, InMemoryUploadedFile) or isinstance(file, TemporaryUploadedFile):
-                        variables = json.loads(file.readline().strip())['allvariables']
+                        variables = None
+                        while line := file.readline().strip():
+                            try:
+                                variables = json.loads(line)['allvariables']
+                                break
+                            except (KeyError, json.JSONDecodeError):
+                                continue
+                        if not variables:
+                            raise Exception("File content missing  build variables")
                         params = namedtuple('ConfigParams', ['observe_only'])(True)
-                        player = EventPlayer(file, variables)
+                        player = eventreplay.EventPlayer(file, variables)
                         if not os.path.exists('{}/{}'.format(logs_dir, file.name)):
                             fs = FileSystemStorage(location=logs_dir)
                             fs.save(file.name, file)
@@ -2125,18 +2080,26 @@  class CommandLineBuilds(TemplateView):
                     else:
                         with open("{}/{}".format(logs_dir, file)) as eventfile:
                             # load variables from the first line
-                            variables = json.loads(eventfile.readline().strip())['allvariables']
+                            variables = None
+                            while line := eventfile.readline().strip():
+                                try:
+                                    variables = json.loads(line)['allvariables']
+                                    break
+                                except (KeyError, json.JSONDecodeError):
+                                    continue
+                            if not variables:
+                                raise Exception("File content missing  build variables")
                             params = namedtuple('ConfigParams', ['observe_only'])(True)
-                            player = EventPlayer(eventfile, variables)
+                            player = eventreplay.EventPlayer(eventfile, variables)
                             toasterui.main(player, player, params)
                     event_log_import = EventLogsImports.objects.create(name=file, imported=True)
                     event_log_import.build_id = Build.objects.last().id
                     event_log_import.save()
                     request.session['file'] = ""
-        except json.decoder.JSONDecodeError:
+        except Exception:
             messages.add_message(
                 self.request,
-                messages.SUCCESS,
+                messages.ERROR,
                 "The file content is not in the correct format. Update file content or upload a different file."
             )
             return HttpResponseRedirect("/toastergui/cmdline/")