[PATCH 00/12] prserv: add support for an "upstream" server

[PATCH 00/12] prserv: add support for an "upstream" server

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

This makes it possible to customize an "upstream" distribution
by modifying local packages. If the "upstream" package bears
revision "x", the local one will have revision "x.y", this
having priority over the upstream one.

Take advantage of this work to clean-up and update the prserv code too.

Michael Opdenacker (12):
  prserv: simplify the PRServerClient() interface
  prserv: use double quotes by default
  bitbake-prserv: replace deprecated optparse by argparse
  prserv: use self.logger instead of logger directly
  asyncrpc: include parse_address from hashserv
  prserv: capitalization and spacing improvements
  prserv: fix read_only test
  prserv: add extra requests
  prserv: remove redundant exception handler
  prserv: correct error message
  prserv: remove unnecessary code
  prserv: add "upstream" server support

 bin/bitbake-prserv        |  99 ++++++++++++-----
 lib/bb/asyncrpc/client.py |  23 ++++
 lib/hashserv/__init__.py  |  27 +----
 lib/prserv/__init__.py    |  21 +++-
 lib/prserv/client.py      |  42 +++++--
 lib/prserv/db.py          | 176 ++++++++++++++++++++---------
 lib/prserv/serv.py        | 228 ++++++++++++++++++++++++++++----------
 7 files changed, 434 insertions(+), 182 deletions(-)

-- 
2.34.1

[PATCH 01/12] prserv: simplify the PRServerClient() interface

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

serv.py: simplify the PRServerClient() interface by passing the
server object instead of multiple arguments, and then retrieving
the data through this object.

This replicates what is done for ServerClient() in hashserv/server.py

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Cc: Joshua Watt <JPEWhacker@gmail.com>
Cc: Tim Orling <ticotimo@gmail.com>
---
 lib/prserv/serv.py | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index 5fc8863f70..8b1bdc0250 100644
--- a/lib/prserv/serv.py
+++ b/lib/prserv/serv.py
@@ -20,16 +20,16 @@ PIDPREFIX = "/tmp/PRServer_%s_%s.pid"
 singleton = None
 
 class PRServerClient(bb.asyncrpc.AsyncServerConnection):
-    def __init__(self, socket, table, read_only):
-        super().__init__(socket, 'PRSERVICE', logger)
+    def __init__(self, socket, server):
+        super().__init__(socket, 'PRSERVICE', server.logger)
+        self.server = server
+
         self.handlers.update({
             'get-pr': self.handle_get_pr,
             'import-one': self.handle_import_one,
             'export': self.handle_export,
             'is-readonly': self.handle_is_readonly,
         })
-        self.table = table
-        self.read_only = read_only
 
     def validate_proto_version(self):
         return (self.proto_version == (1, 0))
@@ -38,10 +38,10 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
         try:
             return await super().dispatch_message(msg)
         except:
-            self.table.sync()
+            self.server.table.sync()
             raise
         else:
-            self.table.sync_if_dirty()
+            self.server.table.sync_if_dirty()
 
     async def handle_get_pr(self, request):
         version = request['version']
@@ -50,7 +50,7 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
 
         response = None
         try:
-            value = self.table.getValue(version, pkgarch, checksum)
+            value = self.server.table.getValue(version, pkgarch, checksum)
             response = {'value': value}
         except prserv.NotFoundError:
             logger.error("can not find value for (%s, %s)",version, checksum)
@@ -67,7 +67,7 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
             checksum = request['checksum']
             value = request['value']
 
-            value = self.table.importone(version, pkgarch, checksum, value)
+            value = self.server.table.importone(version, pkgarch, checksum, value)
             if value is not None:
                 response = {'value': value}
 
@@ -80,7 +80,7 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
         colinfo = request['colinfo']
 
         try:
-            (metainfo, datainfo) = self.table.export(version, pkgarch, checksum, colinfo)
+            (metainfo, datainfo) = self.server.table.export(version, pkgarch, checksum, colinfo)
         except sqlite3.Error as exc:
             logger.error(str(exc))
             metainfo = datainfo = None
@@ -88,7 +88,7 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
         return {'metainfo': metainfo, 'datainfo': datainfo}
 
     async def handle_is_readonly(self, request):
-        return {'readonly': self.read_only}
+        return {'readonly': self.server.read_only}
 
 class PRServer(bb.asyncrpc.AsyncServer):
     def __init__(self, dbfile, read_only=False):
@@ -98,7 +98,7 @@ class PRServer(bb.asyncrpc.AsyncServer):
         self.read_only = read_only
 
     def accept_client(self, socket):
-        return PRServerClient(socket, self.table, self.read_only)
+        return PRServerClient(socket, self)
 
     def start(self):
         tasks = super().start()
-- 
2.34.1

[PATCH 02/12] prserv: use double quotes by default

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

To aligh with the hashserv code

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Cc: Joshua Watt <JPEWhacker@gmail.com>
Cc: Tim Orling <ticotimo@gmail.com>
---
 bin/bitbake-prserv     |  4 +--
 lib/prserv/__init__.py |  4 +--
 lib/prserv/client.py   | 20 ++++++-------
 lib/prserv/db.py       | 32 ++++++++++-----------
 lib/prserv/serv.py     | 64 +++++++++++++++++++++---------------------
 5 files changed, 62 insertions(+), 62 deletions(-)

diff --git a/bin/bitbake-prserv b/bin/bitbake-prserv
index 5be42f3ce5..8c3808fb20 100755
--- a/bin/bitbake-prserv
+++ b/bin/bitbake-prserv
@@ -11,14 +11,14 @@ import optparse
 import warnings
 warnings.simplefilter("default")
 
-sys.path.insert(0, os.path.join(os.path.dirname(os.path.dirname(__file__)),'lib'))
+sys.path.insert(0, os.path.join(os.path.dirname(os.path.dirname(__file__)), "lib"))
 
 import prserv
 import prserv.serv
 
 __version__="1.0.0"
 
-PRHOST_DEFAULT='0.0.0.0'
+PRHOST_DEFAULT="0.0.0.0"
 PRPORT_DEFAULT=8585
 
 def main():
diff --git a/lib/prserv/__init__.py b/lib/prserv/__init__.py
index 38ced818ad..5790a8db1e 100644
--- a/lib/prserv/__init__.py
+++ b/lib/prserv/__init__.py
@@ -12,8 +12,8 @@ import sys,logging
 def init_logger(logfile, loglevel):
     numeric_level = getattr(logging, loglevel.upper(), None)
     if not isinstance(numeric_level, int):
-        raise ValueError('Invalid log level: %s' % loglevel)
-    FORMAT = '%(asctime)-15s %(message)s'
+        raise ValueError("Invalid log level: %s" % loglevel)
+    FORMAT = "%(asctime)-15s %(message)s"
     logging.basicConfig(level=numeric_level, filename=logfile, format=FORMAT)
 
 class NotFoundError(Exception):
diff --git a/lib/prserv/client.py b/lib/prserv/client.py
index 6b81356fac..7bc5188c53 100644
--- a/lib/prserv/client.py
+++ b/lib/prserv/client.py
@@ -11,40 +11,40 @@ logger = logging.getLogger("BitBake.PRserv")
 
 class PRAsyncClient(bb.asyncrpc.AsyncClient):
     def __init__(self):
-        super().__init__('PRSERVICE', '1.0', logger)
+        super().__init__("PRSERVICE", "1.0", logger)
 
     async def getPR(self, version, pkgarch, checksum):
         response = await self.invoke(
-            {'get-pr': {'version': version, 'pkgarch': pkgarch, 'checksum': checksum}}
+            {"get-pr": {"version": version, "pkgarch": pkgarch, "checksum": checksum}}
         )
         if response:
-            return response['value']
+            return response["value"]
 
     async def importone(self, version, pkgarch, checksum, value):
         response = await self.invoke(
-            {'import-one': {'version': version, 'pkgarch': pkgarch, 'checksum': checksum, 'value': value}}
+            {"import-one": {"version": version, "pkgarch": pkgarch, "checksum": checksum, "value": value}}
         )
         if response:
-            return response['value']
+            return response["value"]
 
     async def export(self, version, pkgarch, checksum, colinfo):
         response = await self.invoke(
-            {'export': {'version': version, 'pkgarch': pkgarch, 'checksum': checksum, 'colinfo': colinfo}}
+            {"export": {"version": version, "pkgarch": pkgarch, "checksum": checksum, "colinfo": colinfo}}
         )
         if response:
-            return (response['metainfo'], response['datainfo'])
+            return (response["metainfo"], response["datainfo"])
 
     async def is_readonly(self):
         response = await self.invoke(
-            {'is-readonly': {}}
+            {"is-readonly": {}}
         )
         if response:
-            return response['readonly']
+            return response["readonly"]
 
 class PRClient(bb.asyncrpc.Client):
     def __init__(self):
         super().__init__()
-        self._add_methods('getPR', 'importone', 'export', 'is_readonly')
+        self._add_methods("getPR", "importone", "export", "is_readonly")
 
     def _get_async_client(self):
         return PRAsyncClient()
diff --git a/lib/prserv/db.py b/lib/prserv/db.py
index b4bda7078c..0859cf4f2c 100644
--- a/lib/prserv/db.py
+++ b/lib/prserv/db.py
@@ -64,7 +64,7 @@ class PRTable(object):
             try:
                 return self.conn.execute(*query)
             except sqlite3.OperationalError as exc:
-                if 'is locked' in str(exc) and end > time.time():
+                if "is locked" in str(exc) and end > time.time():
                     continue
                 raise exc
 
@@ -220,18 +220,18 @@ class PRTable(object):
         metainfo = {}
         #column info 
         if colinfo:
-            metainfo['tbl_name'] = self.table
-            metainfo['core_ver'] = prserv.__version__
-            metainfo['col_info'] = []
+            metainfo["tbl_name"] = self.table
+            metainfo["core_ver"] = prserv.__version__
+            metainfo["col_info"] = []
             data = self._execute("PRAGMA table_info(%s);" % self.table)
             for row in data:
                 col = {}
-                col['name'] = row['name']
-                col['type'] = row['type']
-                col['notnull'] = row['notnull']
-                col['dflt_value'] = row['dflt_value']
-                col['pk'] = row['pk']
-                metainfo['col_info'].append(col)
+                col["name"] = row["name"]
+                col["type"] = row["type"]
+                col["notnull"] = row["notnull"]
+                col["dflt_value"] = row["dflt_value"]
+                col["pk"] = row["pk"]
+                metainfo["col_info"].append(col)
 
         #data info
         datainfo = []
@@ -261,12 +261,12 @@ class PRTable(object):
         else:
             data = self._execute(sqlstmt)
         for row in data:
-            if row['version']:
+            if row["version"]:
                 col = {}
-                col['version'] = row['version']
-                col['pkgarch'] = row['pkgarch']
-                col['checksum'] = row['checksum']
-                col['value'] = row['value']
+                col["version"] = row["version"]
+                col["pkgarch"] = row["pkgarch"]
+                col["checksum"] = row["checksum"]
+                col["value"] = row["value"]
                 datainfo.append(col)
         return (metainfo, datainfo)
 
@@ -275,7 +275,7 @@ class PRTable(object):
         for line in self.conn.iterdump():
             writeCount = writeCount + len(line) + 1
             fd.write(line)
-            fd.write('\n')
+            fd.write("\n")
         return writeCount
 
 class PRData(object):
diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index 8b1bdc0250..874edd7069 100644
--- a/lib/prserv/serv.py
+++ b/lib/prserv/serv.py
@@ -21,14 +21,14 @@ singleton = None
 
 class PRServerClient(bb.asyncrpc.AsyncServerConnection):
     def __init__(self, socket, server):
-        super().__init__(socket, 'PRSERVICE', server.logger)
+        super().__init__(socket, "PRSERVICE", server.logger)
         self.server = server
 
         self.handlers.update({
-            'get-pr': self.handle_get_pr,
-            'import-one': self.handle_import_one,
-            'export': self.handle_export,
-            'is-readonly': self.handle_is_readonly,
+            "get-pr": self.handle_get_pr,
+            "import-one": self.handle_import_one,
+            "export": self.handle_export,
+            "is-readonly": self.handle_is_readonly,
         })
 
     def validate_proto_version(self):
@@ -44,14 +44,14 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
             self.server.table.sync_if_dirty()
 
     async def handle_get_pr(self, request):
-        version = request['version']
-        pkgarch = request['pkgarch']
-        checksum = request['checksum']
+        version = request["version"]
+        pkgarch = request["pkgarch"]
+        checksum = request["checksum"]
 
         response = None
         try:
             value = self.server.table.getValue(version, pkgarch, checksum)
-            response = {'value': value}
+            response = {"value": value}
         except prserv.NotFoundError:
             logger.error("can not find value for (%s, %s)",version, checksum)
         except sqlite3.Error as exc:
@@ -62,22 +62,22 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
     async def handle_import_one(self, request):
         response = None
         if not self.read_only:
-            version = request['version']
-            pkgarch = request['pkgarch']
-            checksum = request['checksum']
-            value = request['value']
+            version = request["version"]
+            pkgarch = request["pkgarch"]
+            checksum = request["checksum"]
+            value = request["value"]
 
             value = self.server.table.importone(version, pkgarch, checksum, value)
             if value is not None:
-                response = {'value': value}
+                response = {"value": value}
 
         return response
 
     async def handle_export(self, request):
-        version = request['version']
-        pkgarch = request['pkgarch']
-        checksum = request['checksum']
-        colinfo = request['colinfo']
+        version = request["version"]
+        pkgarch = request["pkgarch"]
+        checksum = request["checksum"]
+        colinfo = request["colinfo"]
 
         try:
             (metainfo, datainfo) = self.server.table.export(version, pkgarch, checksum, colinfo)
@@ -85,10 +85,10 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
             logger.error(str(exc))
             metainfo = datainfo = None
 
-        return {'metainfo': metainfo, 'datainfo': datainfo}
+        return {"metainfo": metainfo, "datainfo": datainfo}
 
     async def handle_is_readonly(self, request):
-        return {'readonly': self.server.read_only}
+        return {"readonly": self.server.read_only}
 
 class PRServer(bb.asyncrpc.AsyncServer):
     def __init__(self, dbfile, read_only=False):
@@ -135,7 +135,7 @@ class PRServSingleton(object):
         if not self.prserv.address:
             raise PRServiceConfigError
         if not self.port:
-            self.port = int(self.prserv.address.rsplit(':', 1)[1])
+            self.port = int(self.prserv.address.rsplit(":", 1)[1])
 
 def run_as_daemon(func, pidfile, logfile):
     """
@@ -171,12 +171,12 @@ def run_as_daemon(func, pidfile, logfile):
     # stdout/stderr or it could be 'real' unix fd forking where we need
     # to physically close the fds to prevent the program launching us from
     # potentially hanging on a pipe. Handle both cases.
-    si = open('/dev/null', 'r')
+    si = open("/dev/null", "r")
     try:
         os.dup2(si.fileno(),sys.stdin.fileno())
     except (AttributeError, io.UnsupportedOperation):
         sys.stdin = si
-    so = open(logfile, 'a+')
+    so = open(logfile, "a+")
     try:
         os.dup2(so.fileno(),sys.stdout.fileno())
     except (AttributeError, io.UnsupportedOperation):
@@ -200,7 +200,7 @@ def run_as_daemon(func, pidfile, logfile):
 
     # write pidfile
     pid = str(os.getpid())
-    with open(pidfile, 'w') as pf:
+    with open(pidfile, "w") as pf:
         pf.write("%s\n" % pid)
 
     func()
@@ -245,12 +245,12 @@ def stop_daemon(host, port):
         # so at least advise the user which ports the corresponding server is listening
         ports = []
         portstr = ""
-        for pf in glob.glob(PIDPREFIX % (ip,'*')):
+        for pf in glob.glob(PIDPREFIX % (ip, "*")):
             bn = os.path.basename(pf)
             root, _ = os.path.splitext(bn)
-            ports.append(root.split('_')[-1])
+            ports.append(root.split("_")[-1])
         if len(ports):
-            portstr = "Wrong port? Other ports listening at %s: %s" % (host, ' '.join(ports))
+            portstr = "Wrong port? Other ports listening at %s: %s" % (host, " ".join(ports))
 
         sys.stderr.write("pidfile %s does not exist. Daemon not running? %s\n"
                          % (pidfile,portstr))
@@ -284,7 +284,7 @@ def is_running(pid):
     return True
 
 def is_local_special(host, port):
-    if (host == 'localhost' or host == '127.0.0.1') and not port:
+    if (host == "localhost" or host == "127.0.0.1") and not port:
         return True
     else:
         return False
@@ -295,7 +295,7 @@ class PRServiceConfigError(Exception):
 def auto_start(d):
     global singleton
 
-    host_params = list(filter(None, (d.getVar('PRSERV_HOST') or '').split(':')))
+    host_params = list(filter(None, (d.getVar("PRSERV_HOST") or "").split(":")))
     if not host_params:
         # Shutdown any existing PR Server
         auto_shutdown()
@@ -304,7 +304,7 @@ def auto_start(d):
     if len(host_params) != 2:
         # Shutdown any existing PR Server
         auto_shutdown()
-        logger.critical('\n'.join(['PRSERV_HOST: incorrect format',
+        logger.critical("\n".join(["PRSERV_HOST: incorrect format",
                 'Usage: PRSERV_HOST = "<hostname>:<port>"']))
         raise PRServiceConfigError
 
@@ -357,8 +357,8 @@ def connect(host, port):
 
     global singleton
 
-    if host.strip().lower() == 'localhost' and not port:
-        host = 'localhost'
+    if host.strip().lower() == "localhost" and not port:
+        host = "localhost"
         port = singleton.port
 
     conn = client.PRClient()
-- 
2.34.1

[PATCH 03/12] bitbake-prserv: replace deprecated optparse by argparse

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

optparse is deprecated since Python 2.7

Note that this is neither supposed to change the options
supported by bitbake-prserv nor the way they are interpreted.

Note that in the "--help" output, long options are now reported
for example as "--host HOST" instead of "--host=HOST" but
both are equivalent anyway, as they already were with optparse.

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Cc: Joshua Watt <JPEWhacker@gmail.com>
Cc: Tim Orling <ticotimo@gmail.com>
---
 bin/bitbake-prserv | 82 +++++++++++++++++++++++++++++++---------------
 1 file changed, 55 insertions(+), 27 deletions(-)

diff --git a/bin/bitbake-prserv b/bin/bitbake-prserv
index 8c3808fb20..ad0a069401 100755
--- a/bin/bitbake-prserv
+++ b/bin/bitbake-prserv
@@ -7,7 +7,7 @@
 
 import os
 import sys,logging
-import optparse
+import argparse
 import warnings
 warnings.simplefilter("default")
 
@@ -16,40 +16,68 @@ sys.path.insert(0, os.path.join(os.path.dirname(os.path.dirname(__file__)), "lib
 import prserv
 import prserv.serv
 
-__version__="1.0.0"
+VERSION = "1.1.0"
 
 PRHOST_DEFAULT="0.0.0.0"
 PRPORT_DEFAULT=8585
 
 def main():
-    parser = optparse.OptionParser(
-        version="Bitbake PR Service Core version %s, %%prog version %s" % (prserv.__version__, __version__),
-        usage = "%prog < --start | --stop > [options]")
+    parser = argparse.ArgumentParser(
+        description="BitBake PR Server. Version=%s" % VERSION,
+        formatter_class=argparse.RawTextHelpFormatter)
 
-    parser.add_option("-f", "--file", help="database filename(default: prserv.sqlite3)", action="store",
-                      dest="dbfile", type="string", default="prserv.sqlite3")
-    parser.add_option("-l", "--log", help="log filename(default: prserv.log)", action="store",
-                      dest="logfile", type="string", default="prserv.log")
-    parser.add_option("--loglevel", help="logging level, i.e. CRITICAL, ERROR, WARNING, INFO, DEBUG",
-                      action = "store", type="string", dest="loglevel", default = "INFO")
-    parser.add_option("--start", help="start daemon",
-                      action="store_true", dest="start")
-    parser.add_option("--stop", help="stop daemon",
-                      action="store_true", dest="stop")
-    parser.add_option("--host", help="ip address to bind", action="store",
-                      dest="host", type="string", default=PRHOST_DEFAULT)
-    parser.add_option("--port", help="port number(default: 8585)", action="store",
-                      dest="port", type="int", default=PRPORT_DEFAULT)
-    parser.add_option("-r", "--read-only", help="open database in read-only mode",
-                      action="store_true")
+    parser.add_argument(
+        "-f",
+        "--file",
+        default="prserv.sqlite3",
+        help="database filename (default: prserv.sqlite3)",
+    )
+    parser.add_argument(
+        "-l",
+        "--log",
+        default="prserv.log",
+        help="log filename(default: prserv.log)",
+    )
+    parser.add_argument(
+        "--loglevel",
+        default="INFO",
+        help="logging level, i.e. CRITICAL, ERROR, WARNING, INFO, DEBUG",
+    )
+    parser.add_argument(
+        "--start",
+        action="store_true",
+        help="start daemon",
+    )
+    parser.add_argument(
+        "--stop",
+        action="store_true",
+        help="stop daemon",
+    )
+    parser.add_argument(
+        "--host",
+        help="ip address to bind",
+        default=PRHOST_DEFAULT,
+    )
+    parser.add_argument(
+        "--port",
+        type=int,
+        default=PRPORT_DEFAULT,
+        help="port number (default: 8585)",
+    )
+    parser.add_argument(
+        "-r",
+        "--read-only",
+        action="store_true",
+        help="open database in read-only mode",
+    )
 
-    options, args = parser.parse_args(sys.argv)
-    prserv.init_logger(os.path.abspath(options.logfile),options.loglevel)
+    args = parser.parse_args()
+    prserv.init_logger(os.path.abspath(args.log), args.loglevel)
 
-    if options.start:
-        ret=prserv.serv.start_daemon(options.dbfile, options.host, options.port,os.path.abspath(options.logfile), options.read_only)
-    elif options.stop:
-        ret=prserv.serv.stop_daemon(options.host, options.port)
+    if args.start:
+        ret=prserv.serv.start_daemon(args.file, args.host, args.port, os.path.abspath(args.log), args.read_only)
+    elif args.stop:
+        ret=prserv.serv.stop_daemon(args.host, args.port)
     else:
         ret=parser.print_help()
     return ret
-- 
2.34.1

[PATCH 04/12] prserv: use self.logger instead of logger directly

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

In both the PRServerClient and PRClient objects.

This aligns with what is done in hashserv/server.py and makes it
possible to benefit from possible specializations of the logger
in the corresponding super classes, instead of using
always the global logger.

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Cc: Joshua Watt <JPEWhacker@gmail.com>
Cc: Tim Orling <ticotimo@gmail.com>
---
 lib/prserv/serv.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index 874edd7069..eafa76673c 100644
--- a/lib/prserv/serv.py
+++ b/lib/prserv/serv.py
@@ -53,9 +53,9 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
             value = self.server.table.getValue(version, pkgarch, checksum)
             response = {"value": value}
         except prserv.NotFoundError:
-            logger.error("can not find value for (%s, %s)",version, checksum)
+            self.logger.error("can not find value for (%s, %s)",version, checksum)
         except sqlite3.Error as exc:
-            logger.error(str(exc))
+            self.logger.error(str(exc))
 
         return response
 
@@ -82,7 +82,7 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
         try:
             (metainfo, datainfo) = self.server.table.export(version, pkgarch, checksum, colinfo)
         except sqlite3.Error as exc:
-            logger.error(str(exc))
+            self.logger.error(str(exc))
             metainfo = datainfo = None
 
         return {"metainfo": metainfo, "datainfo": datainfo}
@@ -105,7 +105,7 @@ class PRServer(bb.asyncrpc.AsyncServer):
         self.db = prserv.db.PRData(self.dbfile, read_only=self.read_only)
         self.table = self.db["PRMAIN"]
 
-        logger.info("Started PRServer with DBfile: %s, Address: %s, PID: %s" %
+        self.logger.info("Started PRServer with DBfile: %s, Address: %s, PID: %s" %
                      (self.dbfile, self.address, str(os.getpid())))
 
         return tasks
-- 
2.34.1

[PATCH 05/12] asyncrpc: include parse_address from hashserv

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

Moving the code and related definitions from
hashserv/__init__.py to asyncrpc/client.py,
allowing this function to be used in other asyncrpc clients.

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Suggested-by: Joshua Watt <JPEWhacker@gmail.com>
Cc: Tim Orling <ticotimo@gmail.com>
---
 lib/bb/asyncrpc/client.py | 23 +++++++++++++++++++++++
 lib/hashserv/__init__.py  | 27 +--------------------------
 2 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/lib/bb/asyncrpc/client.py b/lib/bb/asyncrpc/client.py
index 29a5ab76aa..a350b4fb12 100644
--- a/lib/bb/asyncrpc/client.py
+++ b/lib/bb/asyncrpc/client.py
@@ -10,11 +10,34 @@ import json
 import os
 import socket
 import sys
+import re
 import contextlib
 from threading import Thread
 from .connection import StreamConnection, WebsocketConnection, DEFAULT_MAX_CHUNK
 from .exceptions import ConnectionClosedError, InvokeError
 
+UNIX_PREFIX = "unix://"
+WS_PREFIX = "ws://"
+WSS_PREFIX = "wss://"
+
+ADDR_TYPE_UNIX = 0
+ADDR_TYPE_TCP = 1
+ADDR_TYPE_WS = 2
+
+def parse_address(addr):
+    if addr.startswith(UNIX_PREFIX):
+        return (ADDR_TYPE_UNIX, (addr[len(UNIX_PREFIX) :],))
+    elif addr.startswith(WS_PREFIX) or addr.startswith(WSS_PREFIX):
+        return (ADDR_TYPE_WS, (addr,))
+    else:
+        m = re.match(r"\[(?P<host>[^\]]*)\]:(?P<port>\d+)$", addr)
+        if m is not None:
+            host = m.group("host")
+            port = m.group("port")
+        else:
+            host, port = addr.split(":")
+
+        return (ADDR_TYPE_TCP, (host, int(port)))
 
 class AsyncClient(object):
     def __init__(
diff --git a/lib/hashserv/__init__.py b/lib/hashserv/__init__.py
index 552a33278f..74367eb6b4 100644
--- a/lib/hashserv/__init__.py
+++ b/lib/hashserv/__init__.py
@@ -5,39 +5,14 @@
 
 import asyncio
 from contextlib import closing
-import re
 import itertools
 import json
 from collections import namedtuple
 from urllib.parse import urlparse
-
-UNIX_PREFIX = "unix://"
-WS_PREFIX = "ws://"
-WSS_PREFIX = "wss://"
-
-ADDR_TYPE_UNIX = 0
-ADDR_TYPE_TCP = 1
-ADDR_TYPE_WS = 2
+from bb.asyncrpc.client import parse_address, ADDR_TYPE_UNIX, ADDR_TYPE_WS
 
 User = namedtuple("User", ("username", "permissions"))
 
-
-def parse_address(addr):
-    if addr.startswith(UNIX_PREFIX):
-        return (ADDR_TYPE_UNIX, (addr[len(UNIX_PREFIX) :],))
-    elif addr.startswith(WS_PREFIX) or addr.startswith(WSS_PREFIX):
-        return (ADDR_TYPE_WS, (addr,))
-    else:
-        m = re.match(r"\[(?P<host>[^\]]*)\]:(?P<port>\d+)$", addr)
-        if m is not None:
-            host = m.group("host")
-            port = m.group("port")
-        else:
-            host, port = addr.split(":")
-
-        return (ADDR_TYPE_TCP, (host, int(port)))
-
-
 def create_server(
     addr,
     dbname,
-- 
2.34.1

[PATCH 06/12] prserv: capitalization and spacing improvements

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

Choosing only one style of capitalization
Add extra space after some commas too
Remove idle spaces

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Cc: Joshua Watt <JPEWhacker@gmail.com>
Cc: Tim Orling <ticotimo@gmail.com>
---
 lib/prserv/__init__.py |  2 +-
 lib/prserv/db.py       | 56 +++++++++++++++++++++---------------------
 lib/prserv/serv.py     | 10 ++++----
 3 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/lib/prserv/__init__.py b/lib/prserv/__init__.py
index 5790a8db1e..0e0aa34d0e 100644
--- a/lib/prserv/__init__.py
+++ b/lib/prserv/__init__.py
@@ -7,7 +7,7 @@
 __version__ = "1.0.0"
 
 import os, time
-import sys,logging
+import sys, logging
 
 def init_logger(logfile, loglevel):
     numeric_level = getattr(logging, loglevel.upper(), None)
diff --git a/lib/prserv/db.py b/lib/prserv/db.py
index 0859cf4f2c..7bc2b2dc2d 100644
--- a/lib/prserv/db.py
+++ b/lib/prserv/db.py
@@ -38,9 +38,9 @@ class PRTable(object):
         self.read_only = read_only
         self.dirty = False
         if nohist:
-            self.table = "%s_nohist" % table 
+            self.table = "%s_nohist" % table
         else:
-            self.table = "%s_hist" % table 
+            self.table = "%s_hist" % table
 
         if self.read_only:
             table_exists = self._execute(
@@ -78,7 +78,7 @@ class PRTable(object):
             self.sync()
             self.dirty = False
 
-    def _getValueHist(self, version, pkgarch, checksum):
+    def _get_value_hist(self, version, pkgarch, checksum):
         data=self._execute("SELECT value FROM %s WHERE version=? AND pkgarch=? AND checksum=?;" % self.table,
                            (version, pkgarch, checksum))
         row=data.fetchone()
@@ -87,7 +87,7 @@ class PRTable(object):
         else:
             #no value found, try to insert
             if self.read_only:
-                data = self._execute("SELECT ifnull(max(value)+1,0) FROM %s where version=? AND pkgarch=?;" % (self.table),
+                data = self._execute("SELECT ifnull(max(value)+1, 0) FROM %s where version=? AND pkgarch=?;" % (self.table),
                                    (version, pkgarch))
                 row = data.fetchone()
                 if row is not None:
@@ -96,9 +96,9 @@ class PRTable(object):
                     return 0
 
             try:
-                self._execute("INSERT INTO %s VALUES (?, ?, ?, (select ifnull(max(value)+1,0) from %s where version=? AND pkgarch=?));"
-                           % (self.table,self.table),
-                           (version,pkgarch, checksum,version, pkgarch))
+                self._execute("INSERT INTO %s VALUES (?, ?, ?, (select ifnull(max(value)+1, 0) from %s where version=? AND pkgarch=?));"
+                           % (self.table, self.table),
+                           (version, pkgarch, checksum, version, pkgarch))
             except sqlite3.IntegrityError as exc:
                 logger.error(str(exc))
 
@@ -112,10 +112,10 @@ class PRTable(object):
             else:
                 raise prserv.NotFoundError
 
-    def _getValueNohist(self, version, pkgarch, checksum):
+    def _get_value_no_hist(self, version, pkgarch, checksum):
         data=self._execute("SELECT value FROM %s \
                             WHERE version=? AND pkgarch=? AND checksum=? AND \
-                            value >= (select max(value) from %s where version=? AND pkgarch=?);" 
+                            value >= (select max(value) from %s where version=? AND pkgarch=?);"
                             % (self.table, self.table),
                             (version, pkgarch, checksum, version, pkgarch))
         row=data.fetchone()
@@ -124,7 +124,7 @@ class PRTable(object):
         else:
             #no value found, try to insert
             if self.read_only:
-                data = self._execute("SELECT ifnull(max(value)+1,0) FROM %s where version=? AND pkgarch=?;" % (self.table),
+                data = self._execute("SELECT ifnull(max(value)+1, 0) FROM %s where version=? AND pkgarch=?;" % (self.table),
                                    (version, pkgarch))
                 row = data.fetchone()
                 if row is not None:
@@ -133,8 +133,8 @@ class PRTable(object):
                     return 0
 
             try:
-                self._execute("INSERT OR REPLACE INTO %s VALUES (?, ?, ?, (select ifnull(max(value)+1,0) from %s where version=? AND pkgarch=?));"
-                               % (self.table,self.table),
+                self._execute("INSERT OR REPLACE INTO %s VALUES (?, ?, ?, (select ifnull(max(value)+1, 0) from %s where version=? AND pkgarch=?));"
+                               % (self.table, self.table),
                                (version, pkgarch, checksum, version, pkgarch))
             except sqlite3.IntegrityError as exc:
                 logger.error(str(exc))
@@ -150,17 +150,17 @@ class PRTable(object):
             else:
                 raise prserv.NotFoundError
 
-    def getValue(self, version, pkgarch, checksum):
+    def get_value(self, version, pkgarch, checksum):
         if self.nohist:
-            return self._getValueNohist(version, pkgarch, checksum)
+            return self._get_value_no_hist(version, pkgarch, checksum)
         else:
-            return self._getValueHist(version, pkgarch, checksum)
+            return self._get_value_hist(version, pkgarch, checksum)
 
-    def _importHist(self, version, pkgarch, checksum, value):
+    def _import_hist(self, version, pkgarch, checksum, value):
         if self.read_only:
             return None
 
-        val = None 
+        val = None
         data = self._execute("SELECT value FROM %s WHERE version=? AND pkgarch=? AND checksum=?;" % self.table,
                            (version, pkgarch, checksum))
         row = data.fetchone()
@@ -183,27 +183,27 @@ class PRTable(object):
                 val = row[0]
         return val
 
-    def _importNohist(self, version, pkgarch, checksum, value):
+    def _import_no_hist(self, version, pkgarch, checksum, value):
         if self.read_only:
             return None
 
         try:
             #try to insert
             self._execute("INSERT INTO %s VALUES (?, ?, ?, ?);"  % (self.table),
-                           (version, pkgarch, checksum,value))
+                           (version, pkgarch, checksum, value))
         except sqlite3.IntegrityError as exc:
             #already have the record, try to update
             try:
-                self._execute("UPDATE %s SET value=? WHERE version=? AND pkgarch=? AND checksum=? AND value<?"  
+                self._execute("UPDATE %s SET value=? WHERE version=? AND pkgarch=? AND checksum=? AND value<?"
                               % (self.table),
-                               (value,version,pkgarch,checksum,value))
+                               (value, version, pkgarch, checksum, value))
             except sqlite3.IntegrityError as exc:
                 logger.error(str(exc))
 
         self.dirty = True
 
         data = self._execute("SELECT value FROM %s WHERE version=? AND pkgarch=? AND checksum=? AND value>=?;" % self.table,
-                            (version,pkgarch,checksum,value))
+                            (version, pkgarch, checksum, value))
         row=data.fetchone()
         if row is not None:
             return row[0]
@@ -212,13 +212,13 @@ class PRTable(object):
 
     def importone(self, version, pkgarch, checksum, value):
         if self.nohist:
-            return self._importNohist(version, pkgarch, checksum, value)
+            return self._import_no_hist(version, pkgarch, checksum, value)
         else:
-            return self._importHist(version, pkgarch, checksum, value)
+            return self._import_hist(version, pkgarch, checksum, value)
 
     def export(self, version, pkgarch, checksum, colinfo):
         metainfo = {}
-        #column info 
+        #column info
         if colinfo:
             metainfo["tbl_name"] = self.table
             metainfo["core_ver"] = prserv.__version__
@@ -238,7 +238,7 @@ class PRTable(object):
 
         if self.nohist:
             sqlstmt = "SELECT T1.version, T1.pkgarch, T1.checksum, T1.value FROM %s as T1, \
-                    (SELECT version,pkgarch,max(value) as maxvalue FROM %s GROUP BY version,pkgarch) as T2 \
+                    (SELECT version, pkgarch, max(value) as maxvalue FROM %s GROUP BY version, pkgarch) as T2 \
                     WHERE T1.version=T2.version AND T1.pkgarch=T2.pkgarch AND T1.value=T2.maxvalue " % (self.table, self.table)
         else:
             sqlstmt = "SELECT * FROM %s as T1 WHERE 1=1 " % self.table
@@ -302,7 +302,7 @@ class PRData(object):
     def disconnect(self):
         self.connection.close()
 
-    def __getitem__(self,tblname):
+    def __getitem__(self, tblname):
         if not isinstance(tblname, str):
             raise TypeError("tblname argument must be a string, not '%s'" %
                             type(tblname))
@@ -316,4 +316,4 @@ class PRData(object):
         if tblname in self._tables:
             del self._tables[tblname]
         logger.info("drop table %s" % (tblname))
-        self.connection.execute("DROP TABLE IF EXISTS %s;" % tblname) 
+        self.connection.execute("DROP TABLE IF EXISTS %s;" % tblname)
diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index eafa76673c..242cd11a5a 100644
--- a/lib/prserv/serv.py
+++ b/lib/prserv/serv.py
@@ -50,7 +50,7 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
 
         response = None
         try:
-            value = self.server.table.getValue(version, pkgarch, checksum)
+            value = self.server.table.get_value(version, pkgarch, checksum)
             response = {"value": value}
         except prserv.NotFoundError:
             self.logger.error("can not find value for (%s, %s)",version, checksum)
@@ -173,16 +173,16 @@ def run_as_daemon(func, pidfile, logfile):
     # potentially hanging on a pipe. Handle both cases.
     si = open("/dev/null", "r")
     try:
-        os.dup2(si.fileno(),sys.stdin.fileno())
+        os.dup2(si.fileno(), sys.stdin.fileno())
     except (AttributeError, io.UnsupportedOperation):
         sys.stdin = si
     so = open(logfile, "a+")
     try:
-        os.dup2(so.fileno(),sys.stdout.fileno())
+        os.dup2(so.fileno(), sys.stdout.fileno())
     except (AttributeError, io.UnsupportedOperation):
         sys.stdout = so
     try:
-        os.dup2(so.fileno(),sys.stderr.fileno())
+        os.dup2(so.fileno(), sys.stderr.fileno())
     except (AttributeError, io.UnsupportedOperation):
         sys.stderr = so
 
@@ -253,7 +253,7 @@ def stop_daemon(host, port):
             portstr = "Wrong port? Other ports listening at %s: %s" % (host, " ".join(ports))
 
         sys.stderr.write("pidfile %s does not exist. Daemon not running? %s\n"
-                         % (pidfile,portstr))
+                         % (pidfile, portstr))
         return 1
 
     try:
-- 
2.34.1

[PATCH 07/12] prserv: fix read_only test

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

read_only is an attribute of the parent server object, not of the client.

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Cc: Joshua Watt <JPEWhacker@gmail.com>
Cc: Tim Orling <ticotimo@gmail.com>
---
 lib/prserv/serv.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index 242cd11a5a..efb2e0cf93 100644
--- a/lib/prserv/serv.py
+++ b/lib/prserv/serv.py
@@ -61,7 +61,7 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
 
     async def handle_import_one(self, request):
         response = None
-        if not self.read_only:
+        if not self.server.read_only:
             version = request["version"]
             pkgarch = request["pkgarch"]
             checksum = request["checksum"]
-- 
2.34.1

[PATCH 08/12] prserv: add extra requests

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

Useful for connecting a PR server to an upstream one

- "test-package" checks whether the specified package
  version and arch is known in the database.

- "test-pr" checks a specified output hash is found in the database.
  Otherwise it returns 'None' instead of a new value.

- "max-package-pr" returns the highest PR number for
  (version, arch) entries in the database, and 0 if not found

Add new DB functions supporting the above, plus test_value()
which tells whether a given value is available for the specified
package and architecture.

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Cc: Joshua Watt <JPEWhacker@gmail.com>
Cc: Tim Orling <ticotimo@gmail.com>
---
 lib/prserv/client.py | 23 ++++++++++++++++++++-
 lib/prserv/db.py     | 48 +++++++++++++++++++++++++++++++++++++++++++-
 lib/prserv/serv.py   | 28 ++++++++++++++++++++++++++
 3 files changed, 97 insertions(+), 2 deletions(-)

diff --git a/lib/prserv/client.py b/lib/prserv/client.py
index 7bc5188c53..8471ee3046 100644
--- a/lib/prserv/client.py
+++ b/lib/prserv/client.py
@@ -20,6 +20,27 @@ class PRAsyncClient(bb.asyncrpc.AsyncClient):
         if response:
             return response["value"]
 
+    async def test_pr(self, version, pkgarch, checksum):
+        response = await self.invoke(
+            {"test-pr": {"version": version, "pkgarch": pkgarch, "checksum": checksum}}
+        )
+        if response:
+            return response["value"]
+
+    async def test_package(self, version, pkgarch):
+        response = await self.invoke(
+            {"test-package": {"version": version, "pkgarch": pkgarch}}
+        )
+        if response:
+            return response["value"]
+
+    async def max_package_pr(self, version, pkgarch):
+        response = await self.invoke(
+            {"max-package-pr": {"version": version, "pkgarch": pkgarch}}
+        )
+        if response:
+            return response["value"]
+
     async def importone(self, version, pkgarch, checksum, value):
         response = await self.invoke(
             {"import-one": {"version": version, "pkgarch": pkgarch, "checksum": checksum, "value": value}}
@@ -44,7 +65,7 @@ class PRAsyncClient(bb.asyncrpc.AsyncClient):
 class PRClient(bb.asyncrpc.Client):
     def __init__(self):
         super().__init__()
-        self._add_methods("getPR", "importone", "export", "is_readonly")
+        self._add_methods("getPR", "test_pr", "test_package", "importone", "export", "is_readonly")
 
     def _get_async_client(self):
         return PRAsyncClient()
diff --git a/lib/prserv/db.py b/lib/prserv/db.py
index 7bc2b2dc2d..14b36ea6c9 100644
--- a/lib/prserv/db.py
+++ b/lib/prserv/db.py
@@ -78,12 +78,58 @@ class PRTable(object):
             self.sync()
             self.dirty = False
 
-    def _get_value_hist(self, version, pkgarch, checksum):
+    def test_package(self, version, pkgarch):
+        """Returns whether the specified package version is found in the database for the specified architecture"""
+
+	# Just returns the value if found or None otherwise
+        data=self._execute("SELECT value FROM %s WHERE version=? AND pkgarch=?;" % self.table,
+                           (version, pkgarch))
+        row=data.fetchone()
+        if row is not None:
+            return True
+        else:
+            return False
+
+    def test_value(self, version, pkgarch, value):
+        """Returns whether the specified value is found in the database for the specified package and architecture"""
+
+	# Just returns the value if found or None otherwise
+        data=self._execute("SELECT value FROM %s WHERE version=? AND pkgarch=? and value=?;" % self.table,
+                           (version, pkgarch, value))
+        row=data.fetchone()
+        if row is not None:
+            return True
+        else:
+            return False
+
+    def find_value(self, version, pkgarch, checksum):
+        """Unlike get_value, just returns the value if found or None otherwise. Doesn't create a new value"""
+
         data=self._execute("SELECT value FROM %s WHERE version=? AND pkgarch=? AND checksum=?;" % self.table,
                            (version, pkgarch, checksum))
         row=data.fetchone()
         if row is not None:
             return row[0]
+        else:
+            return None
+
+    def find_max_value(self, version, pkgarch):
+        """Returns the greatest value for (version, pkgarch), or "0" if not found. Doesn't create a new value"""
+
+        data = self._execute("SELECT max(value) FROM %s where version=? AND pkgarch=?;" % (self.table),
+                             (version, pkgarch))
+        row = data.fetchone()
+        if row is not None:
+            return row[0]
+        else:
+            return "0"
+
+    def _get_value_hist(self, version, pkgarch, checksum):
+
+        val = find_value(self, version, pkgarch, checksum)
+
+        if val is not None:
+            return val
         else:
             #no value found, try to insert
             if self.read_only:
diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index efb2e0cf93..473dfa790c 100644
--- a/lib/prserv/serv.py
+++ b/lib/prserv/serv.py
@@ -26,6 +26,9 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
 
         self.handlers.update({
             "get-pr": self.handle_get_pr,
+            "test-pr": self.handle_test_pr,
+            "test-package": self.handle_test_package,
+            "max-package-pr": self.handle_max_package_pr,
             "import-one": self.handle_import_one,
             "export": self.handle_export,
             "is-readonly": self.handle_is_readonly,
@@ -43,6 +46,31 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
         else:
             self.server.table.sync_if_dirty()
 
+    async def handle_test_pr(self, request):
+        '''Finds the PR value corresponding to the request. If not found, returns None and doesn't insert a new value'''
+        version = request["version"]
+        pkgarch = request["pkgarch"]
+        checksum = request["checksum"]
+
+        value = self.server.table.find_value(version, pkgarch, checksum)
+        return {"value": value}
+
+    async def handle_test_package(self, request):
+        '''Tells whether there are entries for (version, pkgarch) in the db. Returns True or False'''
+        version = request["version"]
+        pkgarch = request["pkgarch"]
+
+        value = self.server.table.test_package(version, pkgarch)
+        return {"value": value}
+
+    async def handle_max_package_pr(self, request):
+        '''Finds the greatest PR value for (version, pkgarch) in the db. Returns 0 if no entry was found'''
+        version = request["version"]
+        pkgarch = request["pkgarch"]
+
+        value = self.server.table.find_max_value(version, pkgarch)
+        return {"value": value}
+
     async def handle_get_pr(self, request):
         version = request["version"]
         pkgarch = request["pkgarch"]
-- 
2.34.1

[PATCH 09/12] prserv: remove redundant exception handler

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

This exception handler is already present in db.py's get_value() code.

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Cc: Joshua Watt <JPEWhacker@gmail.com>
Cc: Tim Orling <ticotimo@gmail.com>
---
 lib/prserv/serv.py | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index 473dfa790c..127036f9cc 100644
--- a/lib/prserv/serv.py
+++ b/lib/prserv/serv.py
@@ -82,8 +82,6 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
             response = {"value": value}
         except prserv.NotFoundError:
             self.logger.error("can not find value for (%s, %s)",version, checksum)
-        except sqlite3.Error as exc:
-            self.logger.error(str(exc))
 
         return response
 
-- 
2.34.1

[PATCH 10/12] prserv: correct error message

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

according to db.py, prserv.NotFoundError is returned here when
adding a new value to the database failed

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Cc: Joshua Watt <JPEWhacker@gmail.com>
Cc: Tim Orling <ticotimo@gmail.com>
---
 lib/prserv/serv.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index 127036f9cc..604df6ce61 100644
--- a/lib/prserv/serv.py
+++ b/lib/prserv/serv.py
@@ -81,7 +81,7 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
             value = self.server.table.get_value(version, pkgarch, checksum)
             response = {"value": value}
         except prserv.NotFoundError:
-            self.logger.error("can not find value for (%s, %s)",version, checksum)
+            self.logger.error("failure storing value in database for (%s, %s)",version, checksum)
 
         return response
 
-- 
2.34.1

[PATCH 11/12] prserv: remove unnecessary code

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

In db.py, the ifnull() statement guarantees that the SQL request will
return a value. It's therefore unnecessary to test the case when no
value is found.

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Cc: Joshua Watt <JPEWhacker@gmail.com>
Cc: Tim Orling <ticotimo@gmail.com>
---
 lib/prserv/db.py | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/lib/prserv/db.py b/lib/prserv/db.py
index 14b36ea6c9..fddea923de 100644
--- a/lib/prserv/db.py
+++ b/lib/prserv/db.py
@@ -135,11 +135,7 @@ class PRTable(object):
             if self.read_only:
                 data = self._execute("SELECT ifnull(max(value)+1, 0) FROM %s where version=? AND pkgarch=?;" % (self.table),
                                    (version, pkgarch))
-                row = data.fetchone()
-                if row is not None:
-                    return row[0]
-                else:
-                    return 0
+                return data.fetchone()[0]
 
             try:
                 self._execute("INSERT INTO %s VALUES (?, ?, ?, (select ifnull(max(value)+1, 0) from %s where version=? AND pkgarch=?));"
@@ -172,11 +168,7 @@ class PRTable(object):
             if self.read_only:
                 data = self._execute("SELECT ifnull(max(value)+1, 0) FROM %s where version=? AND pkgarch=?;" % (self.table),
                                    (version, pkgarch))
-                row = data.fetchone()
-                if row is not None:
-                    return row[0]
-                else:
-                    return 0
+                return data.fetchone()[0]
 
             try:
                 self._execute("INSERT OR REPLACE INTO %s VALUES (?, ?, ?, (select ifnull(max(value)+1, 0) from %s where version=? AND pkgarch=?));"
-- 
2.34.1

[PATCH 12/12] prserv: add "upstream" server support

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

Introduce a PRSERVER_UPSTREAM variable that makes the
local PR server connect to an "upstream" one.

This makes it possible to implement local fixes to an
upstream package (revision "x", in a way that gives the local
update priority (revision "x.y").

Set the comments in the handle_get_pr() function in serv.py
for details about the calculation of the local revision.

Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Cc: Joshua Watt <JPEWhacker@gmail.com>
Cc: Tim Orling <ticotimo@gmail.com>
---
 bin/bitbake-prserv     |  15 +++++-
 lib/prserv/__init__.py |  15 ++++++
 lib/prserv/client.py   |   1 +
 lib/prserv/db.py       |  30 ++++++++++++
 lib/prserv/serv.py     | 106 ++++++++++++++++++++++++++++++++++++-----
 5 files changed, 154 insertions(+), 13 deletions(-)

diff --git a/bin/bitbake-prserv b/bin/bitbake-prserv
index ad0a069401..e39d0fba87 100755
--- a/bin/bitbake-prserv
+++ b/bin/bitbake-prserv
@@ -70,12 +70,25 @@ def main():
         action="store_true",
         help="open database in read-only mode",
     )
+    parser.add_argument(
+        "-u",
+        "--upstream",
+        default=os.environ.get("PRSERVER_UPSTREAM", None),
+        help="Upstream PR service (host:port)",
+    )
 
     args = parser.parse_args()
     prserv.init_logger(os.path.abspath(args.log), args.loglevel)
 
     if args.start:
-        ret=prserv.serv.start_daemon(args.file, args.host, args.port, os.path.abspath(args.log), args.read_only)
+        ret=prserv.serv.start_daemon(
+            args.file,
+            args.host,
+            args.port,
+            os.path.abspath(args.log),
+            args.read_only,
+            args.upstream
+        )
     elif args.stop:
         ret=prserv.serv.stop_daemon(args.host, args.port)
     else:
diff --git a/lib/prserv/__init__.py b/lib/prserv/__init__.py
index 0e0aa34d0e..2ee6a28c04 100644
--- a/lib/prserv/__init__.py
+++ b/lib/prserv/__init__.py
@@ -8,6 +8,7 @@ __version__ = "1.0.0"
 
 import os, time
 import sys, logging
+from bb.asyncrpc.client import parse_address, ADDR_TYPE_UNIX, ADDR_TYPE_WS
 
 def init_logger(logfile, loglevel):
     numeric_level = getattr(logging, loglevel.upper(), None)
@@ -18,3 +19,17 @@ def init_logger(logfile, loglevel):
 
 class NotFoundError(Exception):
     pass
+
+async def create_async_client(addr):
+    from . import client
+
+    c = client.PRAsyncClient()
+
+    try:
+        (typ, a) = parse_address(addr)
+        await c.connect_tcp(*a)
+        return c
+
+    except Exception as e:
+        await c.close()
+        raise e
diff --git a/lib/prserv/client.py b/lib/prserv/client.py
index 8471ee3046..89760b6f74 100644
--- a/lib/prserv/client.py
+++ b/lib/prserv/client.py
@@ -6,6 +6,7 @@
 
 import logging
 import bb.asyncrpc
+from . import create_async_client
 
 logger = logging.getLogger("BitBake.PRserv")
 
diff --git a/lib/prserv/db.py b/lib/prserv/db.py
index fddea923de..b581bbf072 100644
--- a/lib/prserv/db.py
+++ b/lib/prserv/db.py
@@ -124,6 +124,36 @@ class PRTable(object):
         else:
             return "0"
 
+    def find_max_value(self, version, pkgarch):
+        """Returns the greatest value for (version, pkgarch), or "0" if not found. Doesn't create a new value"""
+
+        data = self._execute("SELECT max(value) FROM %s where version=? AND pkgarch=?;" % (self.table),
+                             (version, pkgarch))
+        row = data.fetchone()
+        if row is not None:
+            return row[0]
+        else:
+            return "0"
+
+    def find_new_subvalue(self, version, pkgarch, base):
+        """Returns the greatest "<base>.y" value for (version, pkgarch), or "<base>.1" if not found. Doesn't store a new value"""
+	# The code doesn't propose "<base>.0" because it would store it as "<base>" was declared as an integer
+
+        data = self._execute("SELECT ifnull(max(value)+0.1, %s.1) FROM %s where version=? AND pkgarch=? AND value LIKE '%s.%%';" % (base, self.table, base),
+                             (version, pkgarch))
+        return data.fetchone()[0]
+
+    def store_value(self, version, pkgarch, checksum, value):
+        '''Store new value in the database'''
+
+        try:
+            self._execute("INSERT INTO %s VALUES (?, ?, ?, ?);"  % (self.table),
+                       (version, pkgarch, checksum, value))
+        except sqlite3.IntegrityError as exc:
+            logger.error(str(exc))
+
+        self.dirty = True
+
     def _get_value_hist(self, version, pkgarch, checksum):
 
         val = find_value(self, version, pkgarch, checksum)
diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index 604df6ce61..1461e27ad2 100644
--- a/lib/prserv/serv.py
+++ b/lib/prserv/serv.py
@@ -12,6 +12,7 @@ import sqlite3
 import prserv
 import prserv.db
 import errno
+from . import create_async_client
 import bb.asyncrpc
 
 logger = logging.getLogger("BitBake.PRserv")
@@ -77,13 +78,86 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
         checksum = request["checksum"]
 
         response = None
-        try:
-            value = self.server.table.get_value(version, pkgarch, checksum)
-            response = {"value": value}
-        except prserv.NotFoundError:
-            self.logger.error("failure storing value in database for (%s, %s)",version, checksum)
 
-        return response
+        if self.upstream_client is None:
+            try:
+                value = self.server.table.get_value(version, pkgarch, checksum)
+                response = {"value": value}
+            except prserv.NotFoundError:
+                self.logger.error("failure storing value in database for (%s, %s)", version, checksum)
+
+            return response
+
+        # We have an upstream server.
+        # Check whether the local server already knows the requested configuration
+        # Here we use find_value(), not get_value(), because we don't want
+        # to unconditionally add a new generated value to the database. If the configuration
+        # is a new one, the generated value we will add will depend on what's on the upstream server.
+
+        value = self.server.table.find_value(version, pkgarch, checksum)
+
+        if value is not None:
+
+            # The configuration is already known locally. Let's use it.
+
+            return {"value": value}
+
+        # The configuration is a new one for the local server
+        # Let's ask the upstream server whether it knows it
+
+        known_upstream = await self.upstream_client.test_package(version, pkgarch)
+
+        if not known_upstream:
+
+            # The package is not known upstream, must be a local-only package
+            # Let's compute the PR number using the local-only method
+
+            try:
+                value = self.server.table.get_value(version, pkgarch, checksum)
+                response = {"value": value}
+            except prserv.NotFoundError:
+                self.logger.error("failure storing value in database for (%s, %s)", version, checksum)
+
+            return response
+
+        # The package is known upstream, let's ask the upstream server
+        # whether it knows our new output hash
+
+        value = await self.upstream_client.test_pr(version, pkgarch, checksum)
+
+        if value is not None:
+
+            # Upstream knows this output hash, let's store it and use it too.
+
+            if not self.server.read_only:
+                self.server.table.store_value(version, pkgarch, checksum, value)
+            # If the local server is read only, won't be able to store the new
+            # value in the database and will have to keep asking the upstream server
+
+            return {"value": value}
+
+        # The output hash doesn't exist upstream, get the most recent number from upstream (x)
+        # Then, we want to have a new PR value for the local server: x.y
+
+        upstream_max_value = await self.upstream_client.max_package_pr(version, pkgarch)
+        subvalue = self.server.table.find_new_subvalue(version, pkgarch, upstream_max_value)
+
+        if not self.server.read_only:
+            self.server.table.store_value(version, pkgarch, checksum, subvalue)
+
+        return {"value": subvalue}
+
+    async def process_requests(self):
+        if self.server.upstream is not None:
+            self.upstream_client = await create_async_client(self.server.upstream)
+        else:
+            self.upstream_client = None
+
+        try:
+            await super().process_requests()
+        finally:
+            if self.upstream_client is not None:
+                await self.upstream_client.close()
 
     async def handle_import_one(self, request):
         response = None
@@ -117,11 +191,12 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
         return {"readonly": self.server.read_only}
 
 class PRServer(bb.asyncrpc.AsyncServer):
-    def __init__(self, dbfile, read_only=False):
+    def __init__(self, dbfile, read_only=False, upstream=None):
         super().__init__(logger)
         self.dbfile = dbfile
         self.table = None
         self.read_only = read_only
+        self.upstream = upstream
 
     def accept_client(self, socket):
         return PRServerClient(socket, self)
@@ -134,6 +209,9 @@ class PRServer(bb.asyncrpc.AsyncServer):
         self.logger.info("Started PRServer with DBfile: %s, Address: %s, PID: %s" %
                      (self.dbfile, self.address, str(os.getpid())))
 
+        if self.upstream is not None:
+            self.logger.info("And upstream PRServer: %s " % (self.upstream))
+
         return tasks
 
     async def stop(self):
@@ -147,14 +225,15 @@ class PRServer(bb.asyncrpc.AsyncServer):
             self.table.sync()
 
 class PRServSingleton(object):
-    def __init__(self, dbfile, logfile, host, port):
+    def __init__(self, dbfile, logfile, host, port, upstream):
         self.dbfile = dbfile
         self.logfile = logfile
         self.host = host
         self.port = port
+        self.upstream = upstream
 
     def start(self):
-        self.prserv = PRServer(self.dbfile)
+        self.prserv = PRServer(self.dbfile, upstream=self.upstream)
         self.prserv.start_tcp_server(socket.gethostbyname(self.host), self.port)
         self.process = self.prserv.serve_as_process(log_level=logging.WARNING)
 
@@ -233,7 +312,7 @@ def run_as_daemon(func, pidfile, logfile):
     os.remove(pidfile)
     os._exit(0)
 
-def start_daemon(dbfile, host, port, logfile, read_only=False):
+def start_daemon(dbfile, host, port, logfile, read_only=False, upstream=None):
     ip = socket.gethostbyname(host)
     pidfile = PIDPREFIX % (ip, port)
     try:
@@ -249,7 +328,7 @@ def start_daemon(dbfile, host, port, logfile, read_only=False):
 
     dbfile = os.path.abspath(dbfile)
     def daemon_main():
-        server = PRServer(dbfile, read_only=read_only)
+        server = PRServer(dbfile, read_only=read_only, upstream=upstream)
         server.start_tcp_server(ip, port)
         server.serve_forever()
 
@@ -336,6 +415,9 @@ def auto_start(d):
 
     host = host_params[0].strip().lower()
     port = int(host_params[1])
+
+    upstream = d.getVar("PRSERV_UPSTREAM") or None
+
     if is_local_special(host, port):
         import bb.utils
         cachedir = (d.getVar("PERSISTENT_DIR") or d.getVar("CACHE"))
@@ -350,7 +432,7 @@ def auto_start(d):
                auto_shutdown()
         if not singleton:
             bb.utils.mkdirhier(cachedir)
-            singleton = PRServSingleton(os.path.abspath(dbfile), os.path.abspath(logfile), host, port)
+            singleton = PRServSingleton(os.path.abspath(dbfile), os.path.abspath(logfile), host, port, upstream)
             singleton.start()
     if singleton:
         host = singleton.host
-- 
2.34.1

Re: [bitbake-devel] [PATCH 00/12] prserv: add support for an "upstream" server

[Richard Purdie]

On Fri, 2024-03-29 at 15:39 +0100, Michael Opdenacker via
lists.openembedded.org wrote:
> From: Michael Opdenacker <michael.opdenacker@bootlin.com>
> 
> This makes it possible to customize an "upstream" distribution
> by modifying local packages. If the "upstream" package bears
> revision "x", the local one will have revision "x.y", this
> having priority over the upstream one.
> 
> Take advantage of this work to clean-up and update the prserv code
> too.
> 
> Michael Opdenacker (12):
>   prserv: simplify the PRServerClient() interface
>   prserv: use double quotes by default
>   bitbake-prserv: replace deprecated optparse by argparse
>   prserv: use self.logger instead of logger directly
>   asyncrpc: include parse_address from hashserv
>   prserv: capitalization and spacing improvements
>   prserv: fix read_only test
>   prserv: add extra requests
>   prserv: remove redundant exception handler
>   prserv: correct error message
>   prserv: remove unnecessary code
>   prserv: add "upstream" server support

Most of these series looks like cleanups to make prserv look more like
hashserv, which is fine.

The piece I'm struggling to understand is what happens if we have three
prservs where A has B an upstream and B has C as an upstream. Does the
system cope with multiple levels of increment, x.y.z and so on?

I'm a bit worried we're seeing floats in the results, which suggests
that generic x.y.z wouldn't work?

Also, where do we stand on automated tests for this code?

Cheers,

Richard

Re: [bitbake-devel] [PATCH 00/12] prserv: add support for an "upstream" server

[Michael Opdenacker]

Hi Richard,

Many thanks for the first review!

On 4/3/24 at 17:09, Richard Purdie wrote:
> On Fri, 2024-03-29 at 15:39 +0100, Michael Opdenacker via
> lists.openembedded.org wrote:
>> From: Michael Opdenacker <michael.opdenacker@bootlin.com>
>>
>> This makes it possible to customize an "upstream" distribution
>> by modifying local packages. If the "upstream" package bears
>> revision "x", the local one will have revision "x.y", this
>> having priority over the upstream one.
>>
>> Take advantage of this work to clean-up and update the prserv code
>> too.
>>
>> Michael Opdenacker (12):
>>    prserv: simplify the PRServerClient() interface
>>    prserv: use double quotes by default
>>    bitbake-prserv: replace deprecated optparse by argparse
>>    prserv: use self.logger instead of logger directly
>>    asyncrpc: include parse_address from hashserv
>>    prserv: capitalization and spacing improvements
>>    prserv: fix read_only test
>>    prserv: add extra requests
>>    prserv: remove redundant exception handler
>>    prserv: correct error message
>>    prserv: remove unnecessary code
>>    prserv: add "upstream" server support
> Most of these series looks like cleanups to make prserv look more like
> hashserv, which is fine.
>
> The piece I'm struggling to understand is what happens if we have three
> prservs where A has B an upstream and B has C as an upstream. Does the
> system cope with multiple levels of increment, x.y.z and so on?


I haven't tested that yet, but wasn't sure we wanted to support such as 
multi-level scenario. If you confirm that's within the scope, I can try 
to make this work!

>
> I'm a bit worried we're seeing floats in the results, which suggests
> that generic x.y.z wouldn't work?


I first tried to explicitly convert all the responses from the PR server 
to strings, but that didn't help. Could it be that a return value like 
'0.1' could be automatically interpreted as a float if it can be 
converted to one? There is no explicit conversion to float in the PR 
server code, so it seemed to me that the conversion was done automatically.

>
> Also, where do we stand on automated tests for this code?


Not done yet. I was waiting for the reviews on the first series, but I 
understand that tests will be necessary :)
Cheers
Michael.

-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [bitbake-devel] [PATCH 07/12] prserv: fix read_only test

[Richard Purdie]

On Fri, 2024-03-29 at 15:39 +0100, Michael Opdenacker via
lists.openembedded.org wrote:
> From: Michael Opdenacker <michael.opdenacker@bootlin.com>
> 
> read_only is an attribute of the parent server object, not of the
> client.
> 
> Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
> Cc: Joshua Watt <JPEWhacker@gmail.com>
> Cc: Tim Orling <ticotimo@gmail.com>
> ---
>  lib/prserv/serv.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
> index 242cd11a5a..efb2e0cf93 100644
> --- a/lib/prserv/serv.py
> +++ b/lib/prserv/serv.py
> @@ -61,7 +61,7 @@ class
> PRServerClient(bb.asyncrpc.AsyncServerConnection):
>  
>      async def handle_import_one(self, request):
>          response = None
> -        if not self.read_only:
> +        if not self.server.read_only:
>              version = request["version"]
>              pkgarch = request["pkgarch"]
>              checksum = request["checksum"]
> 

Isn't this a missing piece of "prserv: simplify the PRServerClient()
interface" and should be squashed there?

Cheers,

Richard

Re: [bitbake-devel] [PATCH 00/12] prserv: add support for an "upstream" server

[Richard Purdie]

On Wed, 2024-04-03 at 17:33 +0200, Michael Opdenacker wrote:
> Hi Richard,
> 
> Many thanks for the first review!
> 
> On 4/3/24 at 17:09, Richard Purdie wrote:
> > On Fri, 2024-03-29 at 15:39 +0100, Michael Opdenacker via
> > lists.openembedded.org wrote:
> > > From: Michael Opdenacker <michael.opdenacker@bootlin.com>
> > > 
> > > This makes it possible to customize an "upstream" distribution
> > > by modifying local packages. If the "upstream" package bears
> > > revision "x", the local one will have revision "x.y", this
> > > having priority over the upstream one.
> > > 
> > > Take advantage of this work to clean-up and update the prserv code
> > > too.
> > > 
> > > Michael Opdenacker (12):
> > >    prserv: simplify the PRServerClient() interface
> > >    prserv: use double quotes by default
> > >    bitbake-prserv: replace deprecated optparse by argparse
> > >    prserv: use self.logger instead of logger directly
> > >    asyncrpc: include parse_address from hashserv
> > >    prserv: capitalization and spacing improvements
> > >    prserv: fix read_only test
> > >    prserv: add extra requests
> > >    prserv: remove redundant exception handler
> > >    prserv: correct error message
> > >    prserv: remove unnecessary code
> > >    prserv: add "upstream" server support
> > Most of these series looks like cleanups to make prserv look more like
> > hashserv, which is fine.
> > 
> > The piece I'm struggling to understand is what happens if we have three
> > prservs where A has B an upstream and B has C as an upstream. Does the
> > system cope with multiple levels of increment, x.y.z and so on?
> 
> 
> I haven't tested that yet, but wasn't sure we wanted to support such as 
> multi-level scenario. If you confirm that's within the scope, I can try 
> to make this work!

I believe we will need to support a potential hierarchy of these
servers in the same way hash equivalence can.

> > I'm a bit worried we're seeing floats in the results, which suggests
> > that generic x.y.z wouldn't work?
> 
> 
> I first tried to explicitly convert all the responses from the PR server 
> to strings, but that didn't help. Could it be that a return value like 
> '0.1' could be automatically interpreted as a float if it can be 
> converted to one? There is no explicit conversion to float in the PR 
> server code, so it seemed to me that the conversion was done automatically.

I suspect that it is SQL that is making these floats and you're just
passing the results around from there. The trouble is that whilst X+0.1
will work for x.y, it isn't going to work for x.y.z and beyond. That is
one reason I'm mentioning this now, at same time as we're seening
issues with the float() values.

> > Also, where do we stand on automated tests for this code?
> 
> 
> Not done yet. I was waiting for the reviews on the first series, but I 
> understand that tests will be necessary :)

I would have a bit more confidence if there were some test cases
showing what was working!

Cheers,

Richard

Re: [bitbake-devel] [PATCH 07/12] prserv: fix read_only test

[Michael Opdenacker]

Hi Richard,

On 4/3/24 at 17:42, Richard Purdie wrote:
> On Fri, 2024-03-29 at 15:39 +0100, Michael Opdenacker via
> lists.openembedded.org wrote:
>> From: Michael Opdenacker <michael.opdenacker@bootlin.com>
>>
>> read_only is an attribute of the parent server object, not of the
>> client.
>>
>> Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
>> Cc: Joshua Watt <JPEWhacker@gmail.com>
>> Cc: Tim Orling <ticotimo@gmail.com>
>> ---
>>   lib/prserv/serv.py | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
>> index 242cd11a5a..efb2e0cf93 100644
>> --- a/lib/prserv/serv.py
>> +++ b/lib/prserv/serv.py
>> @@ -61,7 +61,7 @@ class
>> PRServerClient(bb.asyncrpc.AsyncServerConnection):
>>   
>>       async def handle_import_one(self, request):
>>           response = None
>> -        if not self.read_only:
>> +        if not self.server.read_only:
>>               version = request["version"]
>>               pkgarch = request["pkgarch"]
>>               checksum = request["checksum"]
>>
> Isn't this a missing piece of "prserv: simplify the PRServerClient()
> interface" and should be squashed there?

Oops, indeed, very good catch!
Fixed in my branch.
Many thanks!
Michael.

-- 
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [bitbake-devel] [PATCH 07/12] prserv: fix read_only test

[Richard Purdie]

On Wed, 2024-04-03 at 16:42 +0100, Richard Purdie via
lists.openembedded.org wrote:
> On Fri, 2024-03-29 at 15:39 +0100, Michael Opdenacker via
> lists.openembedded.org wrote:
> > From: Michael Opdenacker <michael.opdenacker@bootlin.com>
> > 
> > read_only is an attribute of the parent server object, not of the
> > client.
> > 
> > Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
> > Cc: Joshua Watt <JPEWhacker@gmail.com>
> > Cc: Tim Orling <ticotimo@gmail.com>
> > ---
> >  lib/prserv/serv.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
> > index 242cd11a5a..efb2e0cf93 100644
> > --- a/lib/prserv/serv.py
> > +++ b/lib/prserv/serv.py
> > @@ -61,7 +61,7 @@ class
> > PRServerClient(bb.asyncrpc.AsyncServerConnection):
> >  
> >      async def handle_import_one(self, request):
> >          response = None
> > -        if not self.read_only:
> > +        if not self.server.read_only:
> >              version = request["version"]
> >              pkgarch = request["pkgarch"]
> >              checksum = request["checksum"]
> > 
> 
> Isn't this a missing piece of "prserv: simplify the PRServerClient()
> interface" and should be squashed there?

For testing purposes I squashed this in.

It also shows why I detest things like the quotes patch since that
cosmetic change broke trying to sort out this more important bisection
breaking issue.

Cheers,

Richard

[PATCH 00/12] prserv: add support for an "upstream" server

[michael.opdenacker]

From: Michael Opdenacker <michael.opdenacker@bootlin.com>

This makes it possible to customize an "upstream" distribution
by modifying local packages. If the "upstream" package bears
revision "x", the local one will have revision "x.y", this
having priority over the upstream one.

Multiple levels of upstream servers are supported, so "x.y.z" revisions
are possible too.

Take advantage of this work to clean-up and update the prserv code too.

Manual "sanity" tests have been run so far but automated tests
(based on Python unittest, as for the hash server tests)
are being prepared for the next iteration. They will also to
test also supported scenarios.

Code reviews are welcome, especially for more Pythonic
ways of implementing some details.

---

Changes in V3:

- Revert the commit removing the so far unused "hist" mode, which
  we wish to keep for binary reproducibility sake.

- Simplification of get_value() function to take
  both "hist" and "nohist" modes with the same shared code.

- Add "history" parameter to the "getPR" request,
  so that the client can ask for the mode of its choice.
  This will also make it possible to implement tests
  for both modes.

  Note that more requests ("export", "import"...)
  will also need a "history" parameter, in a future version,
  after the first tests are implemented.

- Several fixes to bugs either present in V2 or introduced
  by the V3 code changes.

- Put all the new features at the tip of the branch,
  to make the cleanup commits easier to merged.

Changes in V2:

- Add this new commit:
  prserv: remove unused "hist" mode in the database backend

- Squash commit "prserv: fix read_only test" into
  commit "prserv: simplify the PRServerClient() interface"
  (Reported by Richard Purdie)

- Fix the code to support increasing "x.y.z" values, thus
  supporting several levels of upstream servers.

- db.py: remove duplicate definition of find_max_value() function in db.py

- prserv.py: remove tabs before comments (Python didn't complain)

- db.py: now stores the revision ("value") as TEXT.
  This way we can store "1.0" without having it transformed to "1"
  when the default type was INTEGER.

- This allows to fix a regression when the first packages were created
  with 'r0.1' instead of 'r0.0' initially.

- find_max_value: now returns None instead of '0' when no value is found
  Before we couldn't tell the difference between a '0'
  max value and the absence of such a value.

Michael Opdenacker (12):
  prserv: simplify the PRServerClient() interface
  prserv: use double quotes by default
  bitbake-prserv: replace deprecated optparse by argparse
  prserv: use self.logger instead of logger directly
  asyncrpc: include parse_address from hashserv
  prserv: capitalization and spacing improvements
  prserv: add extra requests
  prserv: remove redundant exception handler
  prserv: correct error message
  prserv: remove unnecessary code
  prserv: add "upstream" server support
  prserv: add "history" argument to "get-pr" request

 bin/bitbake-prserv        |  99 ++++++++++++-----
 lib/bb/asyncrpc/client.py |  23 ++++
 lib/hashserv/__init__.py  |  27 +----
 lib/prserv/__init__.py    |  21 +++-
 lib/prserv/client.py      |  44 ++++++--
 lib/prserv/db.py          | 225 ++++++++++++++++++++++----------------
 lib/prserv/serv.py        | 220 ++++++++++++++++++++++++++-----------
 7 files changed, 436 insertions(+), 223 deletions(-)

-- 
2.34.1