* [PATCH 01/12] prserv: simplify the PRServerClient() interface
2024-04-12 9:02 [PATCH 00/12] prserv: add support for an "upstream" server michael.opdenacker
@ 2024-04-12 9:02 ` michael.opdenacker
2024-04-12 9:02 ` [PATCH 02/12] prserv: use double quotes by default michael.opdenacker
` (11 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: michael.opdenacker @ 2024-04-12 9:02 UTC (permalink / raw)
To: bitbake-devel
Cc: Michael Opdenacker, Joshua Watt, Tim Orling, Thomas Petazzoni
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>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
lib/prserv/serv.py | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index 5fc8863f70..28af636966 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)
@@ -61,13 +61,13 @@ 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']
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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 02/12] prserv: use double quotes by default
2024-04-12 9:02 [PATCH 00/12] prserv: add support for an "upstream" server michael.opdenacker
2024-04-12 9:02 ` [PATCH 01/12] prserv: simplify the PRServerClient() interface michael.opdenacker
@ 2024-04-12 9:02 ` michael.opdenacker
2024-04-12 9:02 ` [PATCH 03/12] bitbake-prserv: replace deprecated optparse by argparse michael.opdenacker
` (10 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: michael.opdenacker @ 2024-04-12 9:02 UTC (permalink / raw)
To: bitbake-devel
Cc: Michael Opdenacker, Joshua Watt, Tim Orling, Thomas Petazzoni
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>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.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 28af636966..49f5ae36db 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.server.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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 03/12] bitbake-prserv: replace deprecated optparse by argparse
2024-04-12 9:02 [PATCH 00/12] prserv: add support for an "upstream" server michael.opdenacker
2024-04-12 9:02 ` [PATCH 01/12] prserv: simplify the PRServerClient() interface michael.opdenacker
2024-04-12 9:02 ` [PATCH 02/12] prserv: use double quotes by default michael.opdenacker
@ 2024-04-12 9:02 ` michael.opdenacker
2024-04-12 9:02 ` [PATCH 04/12] prserv: use self.logger instead of logger directly michael.opdenacker
` (9 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: michael.opdenacker @ 2024-04-12 9:02 UTC (permalink / raw)
To: bitbake-devel
Cc: Michael Opdenacker, Joshua Watt, Tim Orling, Thomas Petazzoni
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>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 04/12] prserv: use self.logger instead of logger directly
2024-04-12 9:02 [PATCH 00/12] prserv: add support for an "upstream" server michael.opdenacker
` (2 preceding siblings ...)
2024-04-12 9:02 ` [PATCH 03/12] bitbake-prserv: replace deprecated optparse by argparse michael.opdenacker
@ 2024-04-12 9:02 ` michael.opdenacker
2024-04-12 9:02 ` [PATCH 05/12] asyncrpc: include parse_address from hashserv michael.opdenacker
` (8 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: michael.opdenacker @ 2024-04-12 9:02 UTC (permalink / raw)
To: bitbake-devel
Cc: Michael Opdenacker, Joshua Watt, Tim Orling, Thomas Petazzoni
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>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.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 49f5ae36db..ab9a0a66df 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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 05/12] asyncrpc: include parse_address from hashserv
2024-04-12 9:02 [PATCH 00/12] prserv: add support for an "upstream" server michael.opdenacker
` (3 preceding siblings ...)
2024-04-12 9:02 ` [PATCH 04/12] prserv: use self.logger instead of logger directly michael.opdenacker
@ 2024-04-12 9:02 ` michael.opdenacker
2024-04-12 9:02 ` [PATCH 06/12] prserv: capitalization and spacing improvements michael.opdenacker
` (7 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: michael.opdenacker @ 2024-04-12 9:02 UTC (permalink / raw)
To: bitbake-devel
Cc: Michael Opdenacker, Joshua Watt, Tim Orling, Thomas Petazzoni
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>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 06/12] prserv: capitalization and spacing improvements
2024-04-12 9:02 [PATCH 00/12] prserv: add support for an "upstream" server michael.opdenacker
` (4 preceding siblings ...)
2024-04-12 9:02 ` [PATCH 05/12] asyncrpc: include parse_address from hashserv michael.opdenacker
@ 2024-04-12 9:02 ` michael.opdenacker
2024-04-12 9:02 ` [PATCH 07/12] prserv: add extra requests michael.opdenacker
` (6 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: michael.opdenacker @ 2024-04-12 9:02 UTC (permalink / raw)
To: bitbake-devel
Cc: Michael Opdenacker, Joshua Watt, Tim Orling, Thomas Petazzoni
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>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.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 ab9a0a66df..efb2e0cf93 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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 07/12] prserv: add extra requests
2024-04-12 9:02 [PATCH 00/12] prserv: add support for an "upstream" server michael.opdenacker
` (5 preceding siblings ...)
2024-04-12 9:02 ` [PATCH 06/12] prserv: capitalization and spacing improvements michael.opdenacker
@ 2024-04-12 9:02 ` michael.opdenacker
2024-04-17 14:45 ` [bitbake-devel] " Bruce Ashfield
2024-04-12 9:02 ` [PATCH 08/12] prserv: remove redundant exception handler michael.opdenacker
` (5 subsequent siblings)
12 siblings, 1 reply; 20+ messages in thread
From: michael.opdenacker @ 2024-04-12 9:02 UTC (permalink / raw)
To: bitbake-devel
Cc: Michael Opdenacker, Joshua Watt, Tim Orling, Thomas Petazzoni
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 None 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>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
lib/prserv/client.py | 23 +++++++++++++++++++++-
lib/prserv/db.py | 46 ++++++++++++++++++++++++++++++++++++++++++++
lib/prserv/serv.py | 28 +++++++++++++++++++++++++++
3 files changed, 96 insertions(+), 1 deletion(-)
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..7c200602ed 100644
--- a/lib/prserv/db.py
+++ b/lib/prserv/db.py
@@ -78,6 +78,52 @@ class PRTable(object):
self.sync()
self.dirty = False
+ 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):
+ """Returns the value for the specified checksum if found or None otherwise."""
+
+ 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 None 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 None
+
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))
diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index efb2e0cf93..86bd3bb75f 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 None 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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [bitbake-devel] [PATCH 07/12] prserv: add extra requests
2024-04-12 9:02 ` [PATCH 07/12] prserv: add extra requests michael.opdenacker
@ 2024-04-17 14:45 ` Bruce Ashfield
0 siblings, 0 replies; 20+ messages in thread
From: Bruce Ashfield @ 2024-04-17 14:45 UTC (permalink / raw)
To: michael.opdenacker
Cc: bitbake-devel, Joshua Watt, Tim Orling, Thomas Petazzoni
[-- Attachment #1: Type: text/plain, Size: 8694 bytes --]
On Fri, Apr 12, 2024 at 5:02 AM Michael Opdenacker via
lists.openembedded.org <michael.opdenacker=
bootlin.com@lists.openembedded.org> wrote:
> 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 None 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.
>
I always bounce around in my own code on whether I prefer
the "action" or the "object" to be more prominent in my method
naming.
i.e. pr_get or get_pr
I tend to land on pr_get() -style most of the time.
That being said, your routines match up wth the existing style
so my comment is literally that .. commentary. And I have no
concerns with the change.
Reviewed-by: Bruce Ashfield <bruce.ashfield@gmail.com>
>
> Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
> Cc: Joshua Watt <JPEWhacker@gmail.com>
> Cc: Tim Orling <ticotimo@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
> lib/prserv/client.py | 23 +++++++++++++++++++++-
> lib/prserv/db.py | 46 ++++++++++++++++++++++++++++++++++++++++++++
> lib/prserv/serv.py | 28 +++++++++++++++++++++++++++
> 3 files changed, 96 insertions(+), 1 deletion(-)
>
> 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..7c200602ed 100644
> --- a/lib/prserv/db.py
> +++ b/lib/prserv/db.py
> @@ -78,6 +78,52 @@ class PRTable(object):
> self.sync()
> self.dirty = False
>
> + 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):
> + """Returns the value for the specified checksum if found or None
> otherwise."""
> +
> + 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 None 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 None
> +
> 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))
> diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
> index efb2e0cf93..86bd3bb75f 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 None 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
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#16081):
> https://lists.openembedded.org/g/bitbake-devel/message/16081
> Mute This Topic: https://lists.openembedded.org/mt/105479094/1050810
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [
> bruce.ashfield@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
--
- Thou shalt not follow the NULL pointer, for chaos and madness await thee
at its end
- "Use the force Harry" - Gandalf, Star Trek II
[-- Attachment #2: Type: text/html, Size: 12635 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 08/12] prserv: remove redundant exception handler
2024-04-12 9:02 [PATCH 00/12] prserv: add support for an "upstream" server michael.opdenacker
` (6 preceding siblings ...)
2024-04-12 9:02 ` [PATCH 07/12] prserv: add extra requests michael.opdenacker
@ 2024-04-12 9:02 ` michael.opdenacker
2024-04-12 9:02 ` [PATCH 09/12] prserv: correct error message michael.opdenacker
` (4 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: michael.opdenacker @ 2024-04-12 9:02 UTC (permalink / raw)
To: bitbake-devel
Cc: Michael Opdenacker, Joshua Watt, Tim Orling, Thomas Petazzoni
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>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
lib/prserv/serv.py | 2 --
1 file changed, 2 deletions(-)
diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index 86bd3bb75f..d1ce74e066 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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 09/12] prserv: correct error message
2024-04-12 9:02 [PATCH 00/12] prserv: add support for an "upstream" server michael.opdenacker
` (7 preceding siblings ...)
2024-04-12 9:02 ` [PATCH 08/12] prserv: remove redundant exception handler michael.opdenacker
@ 2024-04-12 9:02 ` michael.opdenacker
2024-04-12 9:02 ` [PATCH 10/12] prserv: remove unnecessary code michael.opdenacker
` (3 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: michael.opdenacker @ 2024-04-12 9:02 UTC (permalink / raw)
To: bitbake-devel
Cc: Michael Opdenacker, Joshua Watt, Tim Orling, Thomas Petazzoni
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>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.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 d1ce74e066..dc4be5b620 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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 10/12] prserv: remove unnecessary code
2024-04-12 9:02 [PATCH 00/12] prserv: add support for an "upstream" server michael.opdenacker
` (8 preceding siblings ...)
2024-04-12 9:02 ` [PATCH 09/12] prserv: correct error message michael.opdenacker
@ 2024-04-12 9:02 ` michael.opdenacker
2024-04-12 9:02 ` [PATCH 11/12] prserv: add "upstream" server support michael.opdenacker
` (2 subsequent siblings)
12 siblings, 0 replies; 20+ messages in thread
From: michael.opdenacker @ 2024-04-12 9:02 UTC (permalink / raw)
To: bitbake-devel
Cc: Michael Opdenacker, Joshua Watt, Tim Orling, Thomas Petazzoni
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>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
lib/prserv/db.py | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/lib/prserv/db.py b/lib/prserv/db.py
index 7c200602ed..eb41508198 100644
--- a/lib/prserv/db.py
+++ b/lib/prserv/db.py
@@ -172,11 +172,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
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH 11/12] prserv: add "upstream" server support
2024-04-12 9:02 [PATCH 00/12] prserv: add support for an "upstream" server michael.opdenacker
` (9 preceding siblings ...)
2024-04-12 9:02 ` [PATCH 10/12] prserv: remove unnecessary code michael.opdenacker
@ 2024-04-12 9:02 ` michael.opdenacker
2024-04-17 15:19 ` [bitbake-devel] " Bruce Ashfield
2024-04-20 20:55 ` Joshua Watt
2024-04-12 9:02 ` [PATCH 12/12] prserv: add "history" argument to "get-pr" request michael.opdenacker
2024-04-17 15:22 ` [bitbake-devel] [PATCH 00/12] prserv: add support for an "upstream" server Bruce Ashfield
12 siblings, 2 replies; 20+ messages in thread
From: michael.opdenacker @ 2024-04-12 9:02 UTC (permalink / raw)
To: bitbake-devel
Cc: Michael Opdenacker, Joshua Watt, Tim Orling, Thomas Petazzoni
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").
Update the calculation of the new revisions to support the
case when prior revisions are not integers, but have
an "x.y..." format."
Set the comments in the handle_get_pr() function in serv.py
for details about the calculation of the local revision.
Use the newly developed functions to simplify the implementation
of the get_value() function by merging the code of the earlier
_get_value_hist() and _get_value_nohist() functions, removing
redundancy between both functions. This also makes the code easier
to understand.
Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Cc: Joshua Watt <JPEWhacker@gmail.com>
Cc: Tim Orling <ticotimo@gmail.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
---
bin/bitbake-prserv | 15 ++++-
lib/prserv/__init__.py | 15 +++++
lib/prserv/client.py | 1 +
lib/prserv/db.py | 125 ++++++++++++++++++++---------------------
lib/prserv/serv.py | 95 +++++++++++++++++++++++++++----
5 files changed, 174 insertions(+), 77 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 eb41508198..8305238f7a 100644
--- a/lib/prserv/db.py
+++ b/lib/prserv/db.py
@@ -21,6 +21,20 @@ sqlversion = sqlite3.sqlite_version_info
if sqlversion[0] < 3 or (sqlversion[0] == 3 and sqlversion[1] < 3):
raise Exception("sqlite3 version 3.3.0 or later is required.")
+def increase_revision(ver):
+ """Take a revision string such as "1" or "1.2.3" or even a number and increase its last number
+ This fails if the last number is not an integer"""
+
+ fields=str(ver).split('.')
+ last = fields[-1]
+
+ try:
+ val = int(last)
+ except Exception as e:
+ logger.critical("Unable to increase revision value %s: %s" % (ver, e))
+
+ return ".".join(fields[0:-1] + list(str(val + 1)))
+
#
# "No History" mode - for a given query tuple (version, pkgarch, checksum),
# the returned value will be the largest among all the values of the same
@@ -53,7 +67,7 @@ class PRTable(object):
(version TEXT NOT NULL, \
pkgarch TEXT NOT NULL, \
checksum TEXT NOT NULL, \
- value INTEGER, \
+ value TEXT, \
PRIMARY KEY (version, pkgarch, checksum));" % self.table)
def _execute(self, *query):
@@ -119,84 +133,67 @@ class PRTable(object):
data = self._execute("SELECT max(value) FROM %s where version=? AND pkgarch=?;" % (self.table),
(version, pkgarch))
row = data.fetchone()
- if row is not None:
+ # With SELECT max() requests, you have an empty row when there are no values, therefore the test on row[0]
+ if row is not None and row[0] is not None:
return row[0]
else:
return None
- 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()
- if row is not None:
- return row[0]
+ def find_new_subvalue(self, version, pkgarch, base):
+ """Take and increase the greatest "<base>.y" value for (version, pkgarch), or return "<base>.1" if not found.
+ This doesn't store a new value."""
+
+ data = self._execute("SELECT max(value) FROM %s where version=? AND pkgarch=? AND value LIKE '%s.%%';" % (self.table, base),
+ (version, pkgarch))
+ row = data.fetchone()
+ # With SELECT max() requests, you have an empty row when there are no values, therefore the test on row[0]
+ if row is not None and row[0] is not None:
+ return increase_revision(row[0])
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),
- (version, pkgarch))
- row = data.fetchone()
- if row is not None:
- return row[0]
- else:
- return 0
+ return base + ".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))
- except sqlite3.IntegrityError as exc:
- logger.error(str(exc))
+ def store_value(self, version, pkgarch, checksum, value):
+ """Store new value in the database"""
- self.dirty = True
+ try:
+ self._execute("INSERT INTO %s VALUES (?, ?, ?, ?);" % (self.table),
+ (version, pkgarch, checksum, value))
+ except sqlite3.IntegrityError as exc:
+ logger.error(str(exc))
- 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:
- raise prserv.NotFoundError
+ self.dirty = True
- 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=?);"
- % (self.table, self.table),
- (version, pkgarch, checksum, version, pkgarch))
- row=data.fetchone()
- if row is not None:
- return row[0]
- 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),
- (version, pkgarch))
- return data.fetchone()[0]
+ def _get_value(self, version, pkgarch, checksum):
- 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),
- (version, pkgarch, checksum, version, pkgarch))
- except sqlite3.IntegrityError as exc:
- logger.error(str(exc))
- self.conn.rollback()
+ max_value = self.find_max_value(version, pkgarch)
- self.dirty = True
+ if max_value is None:
+ # version, pkgarch completely unknown. Return initial value.
+ return "0"
- 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:
- raise prserv.NotFoundError
+ value = self.find_value(version, pkgarch, checksum)
+
+ if value is None:
+ # version, pkgarch found but not checksum. Create a new value from the maximum one
+ return increase_revision(max_value)
- def get_value(self, version, pkgarch, checksum):
if self.nohist:
- return self._get_value_no_hist(version, pkgarch, checksum)
+ # "no-history" mode: only return a value if that's the maximum one for
+ # the version and architecture, otherwise create a new one.
+ # This means that the value cannot decrement.
+ if value == max_value:
+ return value
+ else:
+ return increase_revision(max_value)
else:
- return self._get_value_hist(version, pkgarch, checksum)
+ # "hist" mode: we found an existing value. We can return it
+ # whether it's the maximum one or not.
+ return value
+
+ def get_value(self, version, pkgarch, checksum):
+ value = self._get_value(version, pkgarch, checksum)
+ self.store_value(version, pkgarch, checksum, value)
+ return value
def _import_hist(self, version, pkgarch, checksum, value):
if self.read_only:
diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index dc4be5b620..9e07a34445 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")
@@ -76,14 +77,76 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
pkgarch = request["pkgarch"]
checksum = request["checksum"]
- response = None
- try:
+ if self.upstream_client is None:
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 {"value": value}
- 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
+
+ value = self.server.table.get_value(version, pkgarch, checksum)
+ return {"value": value}
+
+ # 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 = await self.upstream_client.max_package_pr(version, pkgarch)
+ # Here we know that the package is known upstream, so upstream_max can't be None
+ subvalue = self.server.table.find_new_subvalue(version, pkgarch, upstream_max)
+
+ 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 +180,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 +198,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 +214,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 +301,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 +317,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 +404,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 +421,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
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [bitbake-devel] [PATCH 11/12] prserv: add "upstream" server support
2024-04-12 9:02 ` [PATCH 11/12] prserv: add "upstream" server support michael.opdenacker
@ 2024-04-17 15:19 ` Bruce Ashfield
2024-04-20 20:55 ` Joshua Watt
1 sibling, 0 replies; 20+ messages in thread
From: Bruce Ashfield @ 2024-04-17 15:19 UTC (permalink / raw)
To: michael.opdenacker
Cc: bitbake-devel, Joshua Watt, Tim Orling, Thomas Petazzoni
[-- Attachment #1: Type: text/plain, Size: 21520 bytes --]
On Fri, Apr 12, 2024 at 5:02 AM Michael Opdenacker via
lists.openembedded.org <michael.opdenacker=
bootlin.com@lists.openembedded.org> wrote:
> From: Michael Opdenacker <michael.opdenacker@bootlin.com>
>
> Introduce a PRSERVER_UPSTREAM variable that makes the
> local PR server connect to an "upstream" one.
>
>
I assume we'll eventually put something about the new variable
in template conf files (commented out), similar to the upstream
hasserv ones ?
> 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").
>
Update the calculation of the new revisions to support the
> case when prior revisions are not integers, but have
> an "x.y..." format."
>
> Set the comments in the handle_get_pr() function in serv.py
> for details about the calculation of the local revision.
>
> Use the newly developed functions to simplify the implementation
> of the get_value() function by merging the code of the earlier
> _get_value_hist() and _get_value_nohist() functions, removing
> redundancy between both functions. This also makes the code easier
> to understand.
>
> Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
> Cc: Joshua Watt <JPEWhacker@gmail.com>
> Cc: Tim Orling <ticotimo@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
> bin/bitbake-prserv | 15 ++++-
> lib/prserv/__init__.py | 15 +++++
> lib/prserv/client.py | 1 +
> lib/prserv/db.py | 125 ++++++++++++++++++++---------------------
> lib/prserv/serv.py | 95 +++++++++++++++++++++++++++----
> 5 files changed, 174 insertions(+), 77 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 eb41508198..8305238f7a 100644
> --- a/lib/prserv/db.py
> +++ b/lib/prserv/db.py
> @@ -21,6 +21,20 @@ sqlversion = sqlite3.sqlite_version_info
> if sqlversion[0] < 3 or (sqlversion[0] == 3 and sqlversion[1] < 3):
> raise Exception("sqlite3 version 3.3.0 or later is required.")
>
> +def increase_revision(ver):
> + """Take a revision string such as "1" or "1.2.3" or even a number and
> increase its last number
> + This fails if the last number is not an integer"""
> +
> + fields=str(ver).split('.')
> + last = fields[-1]
> +
> + try:
> + val = int(last)
> + except Exception as e:
> + logger.critical("Unable to increase revision value %s: %s" %
> (ver, e))
> +
>
What's the chain of events that will happen after this log and the return ?
The log
message itself isn't going to tell us much, as the package name, etc,
aren't available
at this point.
Should we be raising another exception ? Something else ? Such that we can
get a better log message from calling code that has more context about what
package or what might have lead to the issue ?
Should processing stop and the build fail ? If we don't have a failure, we
risk
getting a package with the same version and changes being missed. This
could already be the case in other parts of the code, I'm only wondering it
while
reading this specific routine. (and I can never remember if
logger.cirtical()
aborts execution on it's own).
> + return ".".join(fields[0:-1] + list(str(val + 1)))
> +
> #
> # "No History" mode - for a given query tuple (version, pkgarch,
> checksum),
> # the returned value will be the largest among all the values of the same
> @@ -53,7 +67,7 @@ class PRTable(object):
> (version TEXT NOT NULL, \
> pkgarch TEXT NOT NULL, \
> checksum TEXT NOT NULL, \
> - value INTEGER, \
> + value TEXT, \
> PRIMARY KEY (version, pkgarch, checksum));" %
> self.table)
>
> def _execute(self, *query):
> @@ -119,84 +133,67 @@ class PRTable(object):
> data = self._execute("SELECT max(value) FROM %s where version=?
> AND pkgarch=?;" % (self.table),
> (version, pkgarch))
> row = data.fetchone()
> - if row is not None:
> + # With SELECT max() requests, you have an empty row when there
> are no values, therefore the test on row[0]
> + if row is not None and row[0] is not None:
> return row[0]
> else:
> return None
>
> - 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()
> - if row is not None:
> - return row[0]
> + def find_new_subvalue(self, version, pkgarch, base):
> + """Take and increase the greatest "<base>.y" value for (version,
> pkgarch), or return "<base>.1" if not found.
> + This doesn't store a new value."""
> +
> + data = self._execute("SELECT max(value) FROM %s where version=?
> AND pkgarch=? AND value LIKE '%s.%%';" % (self.table, base),
> + (version, pkgarch))
> + row = data.fetchone()
> + # With SELECT max() requests, you have an empty row when there
> are no values, therefore the test on row[0]
> + if row is not None and row[0] is not None:
> + return increase_revision(row[0])
> 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),
> - (version, pkgarch))
> - row = data.fetchone()
> - if row is not None:
> - return row[0]
> - else:
> - return 0
> + return base + ".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))
> - except sqlite3.IntegrityError as exc:
> - logger.error(str(exc))
> + def store_value(self, version, pkgarch, checksum, value):
> + """Store new value in the database"""
>
> - self.dirty = True
> + try:
> + self._execute("INSERT INTO %s VALUES (?, ?, ?, ?);" %
> (self.table),
> + (version, pkgarch, checksum, value))
> + except sqlite3.IntegrityError as exc:
> + logger.error(str(exc))
>
> - 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:
> - raise prserv.NotFoundError
> + self.dirty = True
>
> - 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=?);"
> - % (self.table, self.table),
> - (version, pkgarch, checksum, version,
> pkgarch))
> - row=data.fetchone()
> - if row is not None:
> - return row[0]
> - 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),
> - (version, pkgarch))
> - return data.fetchone()[0]
> + def _get_value(self, version, pkgarch, checksum):
>
> - 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),
> - (version, pkgarch, checksum, version,
> pkgarch))
> - except sqlite3.IntegrityError as exc:
> - logger.error(str(exc))
> - self.conn.rollback()
> + max_value = self.find_max_value(version, pkgarch)
>
> - self.dirty = True
> + if max_value is None:
> + # version, pkgarch completely unknown. Return initial value.
> + return "0"
>
> - 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:
> - raise prserv.NotFoundError
> + value = self.find_value(version, pkgarch, checksum)
> +
> + if value is None:
> + # version, pkgarch found but not checksum. Create a new value
> from the maximum one
> + return increase_revision(max_value)
>
> - def get_value(self, version, pkgarch, checksum):
> if self.nohist:
> - return self._get_value_no_hist(version, pkgarch, checksum)
> + # "no-history" mode: only return a value if that's the
> maximum one for
> + # the version and architecture, otherwise create a new one.
> + # This means that the value cannot decrement.
> + if value == max_value:
> + return value
> + else:
> + return increase_revision(max_value)
> else:
> - return self._get_value_hist(version, pkgarch, checksum)
> + # "hist" mode: we found an existing value. We can return it
> + # whether it's the maximum one or not.
> + return value
> +
> + def get_value(self, version, pkgarch, checksum):
> + value = self._get_value(version, pkgarch, checksum)
> + self.store_value(version, pkgarch, checksum, value)
> + return value
>
> def _import_hist(self, version, pkgarch, checksum, value):
> if self.read_only:
> diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
> index dc4be5b620..9e07a34445 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")
> @@ -76,14 +77,76 @@ class
> PRServerClient(bb.asyncrpc.AsyncServerConnection):
> pkgarch = request["pkgarch"]
> checksum = request["checksum"]
>
> - response = None
> - try:
> + if self.upstream_client is None:
> 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 {"value": value}
>
> - 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)
> +
>
Remind me .. what happens in the case of timeouts or other issues ? Is there
any way to detect it and provide a message ? Right now, it looks like it'll
just
fall through to local PR server functionality. Which I don't know is right
or wrong,
just that there's no way to really tell it happened.
> + 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
> +
> + value = self.server.table.get_value(version, pkgarch,
> checksum)
> + return {"value": value}
> +
> + # 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
>
I assume it is in the find_value() call above where we'll short circuit the
call
to the upstream server if the hash hasn't changed and we've stored it
locally.
> +
> + 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 = await self.upstream_client.max_package_pr(version,
> pkgarch)
> + # Here we know that the package is known upstream, so
> upstream_max can't be None
> + subvalue = self.server.table.find_new_subvalue(version, pkgarch,
> upstream_max)
> +
> + 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 +180,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 +198,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))
> +
>
I noticed there were some direct call to the global logger above. Is the
mixing of logger and self.logger intentional (based on whether or not
the object has a logger .. I assume .. but I wanted to be sure).
Bruce
> return tasks
>
> async def stop(self):
> @@ -147,14 +214,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 +301,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 +317,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 +404,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 +421,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
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#16086):
> https://lists.openembedded.org/g/bitbake-devel/message/16086
> Mute This Topic: https://lists.openembedded.org/mt/105479101/1050810
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [
> bruce.ashfield@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
--
- Thou shalt not follow the NULL pointer, for chaos and madness await thee
at its end
- "Use the force Harry" - Gandalf, Star Trek II
[-- Attachment #2: Type: text/html, Size: 28664 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [bitbake-devel] [PATCH 11/12] prserv: add "upstream" server support
2024-04-12 9:02 ` [PATCH 11/12] prserv: add "upstream" server support michael.opdenacker
2024-04-17 15:19 ` [bitbake-devel] " Bruce Ashfield
@ 2024-04-20 20:55 ` Joshua Watt
1 sibling, 0 replies; 20+ messages in thread
From: Joshua Watt @ 2024-04-20 20:55 UTC (permalink / raw)
To: michael.opdenacker; +Cc: bitbake-devel, Tim Orling, Thomas Petazzoni
On Fri, Apr 12, 2024 at 4:02 AM Michael Opdenacker via
lists.openembedded.org
<michael.opdenacker=bootlin.com@lists.openembedded.org> wrote:
>
> 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").
>
> Update the calculation of the new revisions to support the
> case when prior revisions are not integers, but have
> an "x.y..." format."
>
> Set the comments in the handle_get_pr() function in serv.py
> for details about the calculation of the local revision.
>
> Use the newly developed functions to simplify the implementation
> of the get_value() function by merging the code of the earlier
> _get_value_hist() and _get_value_nohist() functions, removing
> redundancy between both functions. This also makes the code easier
> to understand.
>
> Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
> Cc: Joshua Watt <JPEWhacker@gmail.com>
> Cc: Tim Orling <ticotimo@gmail.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> ---
> bin/bitbake-prserv | 15 ++++-
> lib/prserv/__init__.py | 15 +++++
> lib/prserv/client.py | 1 +
> lib/prserv/db.py | 125 ++++++++++++++++++++---------------------
> lib/prserv/serv.py | 95 +++++++++++++++++++++++++++----
> 5 files changed, 174 insertions(+), 77 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 eb41508198..8305238f7a 100644
> --- a/lib/prserv/db.py
> +++ b/lib/prserv/db.py
> @@ -21,6 +21,20 @@ sqlversion = sqlite3.sqlite_version_info
> if sqlversion[0] < 3 or (sqlversion[0] == 3 and sqlversion[1] < 3):
> raise Exception("sqlite3 version 3.3.0 or later is required.")
>
> +def increase_revision(ver):
> + """Take a revision string such as "1" or "1.2.3" or even a number and increase its last number
> + This fails if the last number is not an integer"""
> +
> + fields=str(ver).split('.')
> + last = fields[-1]
> +
> + try:
> + val = int(last)
> + except Exception as e:
> + logger.critical("Unable to increase revision value %s: %s" % (ver, e))
> +
> + return ".".join(fields[0:-1] + list(str(val + 1)))
> +
> #
> # "No History" mode - for a given query tuple (version, pkgarch, checksum),
> # the returned value will be the largest among all the values of the same
> @@ -53,7 +67,7 @@ class PRTable(object):
> (version TEXT NOT NULL, \
> pkgarch TEXT NOT NULL, \
> checksum TEXT NOT NULL, \
> - value INTEGER, \
> + value TEXT, \
> PRIMARY KEY (version, pkgarch, checksum));" % self.table)
>
> def _execute(self, *query):
> @@ -119,84 +133,67 @@ class PRTable(object):
> data = self._execute("SELECT max(value) FROM %s where version=? AND pkgarch=?;" % (self.table),
> (version, pkgarch))
> row = data.fetchone()
> - if row is not None:
> + # With SELECT max() requests, you have an empty row when there are no values, therefore the test on row[0]
> + if row is not None and row[0] is not None:
> return row[0]
> else:
> return None
>
> - 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()
> - if row is not None:
> - return row[0]
> + def find_new_subvalue(self, version, pkgarch, base):
> + """Take and increase the greatest "<base>.y" value for (version, pkgarch), or return "<base>.1" if not found.
> + This doesn't store a new value."""
> +
> + data = self._execute("SELECT max(value) FROM %s where version=? AND pkgarch=? AND value LIKE '%s.%%';" % (self.table, base),
> + (version, pkgarch))
> + row = data.fetchone()
> + # With SELECT max() requests, you have an empty row when there are no values, therefore the test on row[0]
> + if row is not None and row[0] is not None:
> + return increase_revision(row[0])
> 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),
> - (version, pkgarch))
> - row = data.fetchone()
> - if row is not None:
> - return row[0]
> - else:
> - return 0
> + return base + ".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))
> - except sqlite3.IntegrityError as exc:
> - logger.error(str(exc))
> + def store_value(self, version, pkgarch, checksum, value):
> + """Store new value in the database"""
>
> - self.dirty = True
> + try:
> + self._execute("INSERT INTO %s VALUES (?, ?, ?, ?);" % (self.table),
> + (version, pkgarch, checksum, value))
> + except sqlite3.IntegrityError as exc:
> + logger.error(str(exc))
>
> - 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:
> - raise prserv.NotFoundError
> + self.dirty = True
>
> - 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=?);"
> - % (self.table, self.table),
> - (version, pkgarch, checksum, version, pkgarch))
> - row=data.fetchone()
> - if row is not None:
> - return row[0]
> - 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),
> - (version, pkgarch))
> - return data.fetchone()[0]
> + def _get_value(self, version, pkgarch, checksum):
>
> - 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),
> - (version, pkgarch, checksum, version, pkgarch))
> - except sqlite3.IntegrityError as exc:
> - logger.error(str(exc))
> - self.conn.rollback()
> + max_value = self.find_max_value(version, pkgarch)
>
> - self.dirty = True
> + if max_value is None:
> + # version, pkgarch completely unknown. Return initial value.
> + return "0"
>
> - 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:
> - raise prserv.NotFoundError
> + value = self.find_value(version, pkgarch, checksum)
> +
> + if value is None:
> + # version, pkgarch found but not checksum. Create a new value from the maximum one
> + return increase_revision(max_value)
>
> - def get_value(self, version, pkgarch, checksum):
> if self.nohist:
> - return self._get_value_no_hist(version, pkgarch, checksum)
> + # "no-history" mode: only return a value if that's the maximum one for
> + # the version and architecture, otherwise create a new one.
> + # This means that the value cannot decrement.
> + if value == max_value:
> + return value
> + else:
> + return increase_revision(max_value)
> else:
> - return self._get_value_hist(version, pkgarch, checksum)
> + # "hist" mode: we found an existing value. We can return it
> + # whether it's the maximum one or not.
> + return value
> +
> + def get_value(self, version, pkgarch, checksum):
> + value = self._get_value(version, pkgarch, checksum)
> + self.store_value(version, pkgarch, checksum, value)
> + return value
>
> def _import_hist(self, version, pkgarch, checksum, value):
> if self.read_only:
> diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
> index dc4be5b620..9e07a34445 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")
> @@ -76,14 +77,76 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
> pkgarch = request["pkgarch"]
> checksum = request["checksum"]
>
> - response = None
> - try:
> + if self.upstream_client is None:
> 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 {"value": value}
>
> - 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
> +
> + value = self.server.table.get_value(version, pkgarch, checksum)
> + return {"value": value}
> +
> + # 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 = await self.upstream_client.max_package_pr(version, pkgarch)
> + # Here we know that the package is known upstream, so upstream_max can't be None
> + subvalue = self.server.table.find_new_subvalue(version, pkgarch, upstream_max)
> +
> + if not self.server.read_only:
> + self.server.table.store_value(version, pkgarch, checksum, subvalue)
> +
> + return {"value": subvalue}
Keep in mind that this code is effectively multithreaded. I can't tell
if you'll be having race problems here due to simultaneous accesses,
but and sort of workflows where you read something, then update the
database based on the read value can have weird race conditions,
especially when dealing with many clients at once
> +
> + 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 +180,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 +198,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 +214,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 +301,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 +317,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 +404,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 +421,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
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#16086): https://lists.openembedded.org/g/bitbake-devel/message/16086
> Mute This Topic: https://lists.openembedded.org/mt/105479101/3616693
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [JPEWhacker@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 12/12] prserv: add "history" argument to "get-pr" request
2024-04-12 9:02 [PATCH 00/12] prserv: add support for an "upstream" server michael.opdenacker
` (10 preceding siblings ...)
2024-04-12 9:02 ` [PATCH 11/12] prserv: add "upstream" server support michael.opdenacker
@ 2024-04-12 9:02 ` michael.opdenacker
2024-04-17 15:21 ` [bitbake-devel] " Bruce Ashfield
2024-04-17 15:22 ` [bitbake-devel] [PATCH 00/12] prserv: add support for an "upstream" server Bruce Ashfield
12 siblings, 1 reply; 20+ messages in thread
From: michael.opdenacker @ 2024-04-12 9:02 UTC (permalink / raw)
To: bitbake-devel
Cc: Michael Opdenacker, Thomas Petazzoni, Joshua Watt, Tim Orling
From: Michael Opdenacker <michael.opdenacker@bootlin.com>
This makes it possible to test the "history" mode in the
future bitbake selftests, to make sure that both "history"
and "no history" modes work as expected.
Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Joshua Watt <JPEWhacker@gmail.com>
Cc: Tim Orling <ticotimo@gmail.com>
---
lib/prserv/client.py | 4 ++--
lib/prserv/db.py | 16 ++++++++--------
lib/prserv/serv.py | 5 +++--
3 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/lib/prserv/client.py b/lib/prserv/client.py
index 89760b6f74..389172f055 100644
--- a/lib/prserv/client.py
+++ b/lib/prserv/client.py
@@ -14,9 +14,9 @@ class PRAsyncClient(bb.asyncrpc.AsyncClient):
def __init__(self):
super().__init__("PRSERVICE", "1.0", logger)
- async def getPR(self, version, pkgarch, checksum):
+ async def getPR(self, version, pkgarch, checksum, history=False):
response = await self.invoke(
- {"get-pr": {"version": version, "pkgarch": pkgarch, "checksum": checksum}}
+ {"get-pr": {"version": version, "pkgarch": pkgarch, "checksum": checksum, "history": history}}
)
if response:
return response["value"]
diff --git a/lib/prserv/db.py b/lib/prserv/db.py
index 8305238f7a..d41b781622 100644
--- a/lib/prserv/db.py
+++ b/lib/prserv/db.py
@@ -163,7 +163,7 @@ class PRTable(object):
self.dirty = True
- def _get_value(self, version, pkgarch, checksum):
+ def _get_value(self, version, pkgarch, checksum, history):
max_value = self.find_max_value(version, pkgarch)
@@ -177,7 +177,11 @@ class PRTable(object):
# version, pkgarch found but not checksum. Create a new value from the maximum one
return increase_revision(max_value)
- if self.nohist:
+ if history:
+ # "history" mode: we found an existing value. We can return it
+ # whether it's the maximum one or not.
+ return value
+ else:
# "no-history" mode: only return a value if that's the maximum one for
# the version and architecture, otherwise create a new one.
# This means that the value cannot decrement.
@@ -185,13 +189,9 @@ class PRTable(object):
return value
else:
return increase_revision(max_value)
- else:
- # "hist" mode: we found an existing value. We can return it
- # whether it's the maximum one or not.
- return value
- def get_value(self, version, pkgarch, checksum):
- value = self._get_value(version, pkgarch, checksum)
+ def get_value(self, version, pkgarch, checksum, history):
+ value = self._get_value(version, pkgarch, checksum, history)
self.store_value(version, pkgarch, checksum, value)
return value
diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
index 9e07a34445..f692d050b8 100644
--- a/lib/prserv/serv.py
+++ b/lib/prserv/serv.py
@@ -76,9 +76,10 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
version = request["version"]
pkgarch = request["pkgarch"]
checksum = request["checksum"]
+ history = request["history"]
if self.upstream_client is None:
- value = self.server.table.get_value(version, pkgarch, checksum)
+ value = self.server.table.get_value(version, pkgarch, checksum, history)
return {"value": value}
# We have an upstream server.
@@ -105,7 +106,7 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
# The package is not known upstream, must be a local-only package
# Let's compute the PR number using the local-only method
- value = self.server.table.get_value(version, pkgarch, checksum)
+ value = self.server.table.get_value(version, pkgarch, checksum, history)
return {"value": value}
# The package is known upstream, let's ask the upstream server
--
2.34.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [bitbake-devel] [PATCH 12/12] prserv: add "history" argument to "get-pr" request
2024-04-12 9:02 ` [PATCH 12/12] prserv: add "history" argument to "get-pr" request michael.opdenacker
@ 2024-04-17 15:21 ` Bruce Ashfield
2024-04-17 15:37 ` Michael Opdenacker
0 siblings, 1 reply; 20+ messages in thread
From: Bruce Ashfield @ 2024-04-17 15:21 UTC (permalink / raw)
To: michael.opdenacker
Cc: bitbake-devel, Thomas Petazzoni, Joshua Watt, Tim Orling
[-- Attachment #1: Type: text/plain, Size: 5506 bytes --]
I can't say that I understand the difference between history and no-history
from
just reading this series .. but I realize it isn't something being added as
part of
this feature addition.
I just wanted to comment that from the name "history" alone, I can't figure
out what it is doing and how it impacts PR values.
Bruce
On Fri, Apr 12, 2024 at 5:02 AM Michael Opdenacker via
lists.openembedded.org <michael.opdenacker=
bootlin.com@lists.openembedded.org> wrote:
> From: Michael Opdenacker <michael.opdenacker@bootlin.com>
>
> This makes it possible to test the "history" mode in the
> future bitbake selftests, to make sure that both "history"
> and "no history" modes work as expected.
>
> Signed-off-by: Michael Opdenacker <michael.opdenacker@bootlin.com>
> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Cc: Joshua Watt <JPEWhacker@gmail.com>
> Cc: Tim Orling <ticotimo@gmail.com>
> ---
> lib/prserv/client.py | 4 ++--
> lib/prserv/db.py | 16 ++++++++--------
> lib/prserv/serv.py | 5 +++--
> 3 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/lib/prserv/client.py b/lib/prserv/client.py
> index 89760b6f74..389172f055 100644
> --- a/lib/prserv/client.py
> +++ b/lib/prserv/client.py
> @@ -14,9 +14,9 @@ class PRAsyncClient(bb.asyncrpc.AsyncClient):
> def __init__(self):
> super().__init__("PRSERVICE", "1.0", logger)
>
> - async def getPR(self, version, pkgarch, checksum):
> + async def getPR(self, version, pkgarch, checksum, history=False):
> response = await self.invoke(
> - {"get-pr": {"version": version, "pkgarch": pkgarch,
> "checksum": checksum}}
> + {"get-pr": {"version": version, "pkgarch": pkgarch,
> "checksum": checksum, "history": history}}
> )
> if response:
> return response["value"]
> diff --git a/lib/prserv/db.py b/lib/prserv/db.py
> index 8305238f7a..d41b781622 100644
> --- a/lib/prserv/db.py
> +++ b/lib/prserv/db.py
> @@ -163,7 +163,7 @@ class PRTable(object):
>
> self.dirty = True
>
> - def _get_value(self, version, pkgarch, checksum):
> + def _get_value(self, version, pkgarch, checksum, history):
>
> max_value = self.find_max_value(version, pkgarch)
>
> @@ -177,7 +177,11 @@ class PRTable(object):
> # version, pkgarch found but not checksum. Create a new value
> from the maximum one
> return increase_revision(max_value)
>
> - if self.nohist:
> + if history:
> + # "history" mode: we found an existing value. We can return it
> + # whether it's the maximum one or not.
> + return value
> + else:
> # "no-history" mode: only return a value if that's the
> maximum one for
> # the version and architecture, otherwise create a new one.
> # This means that the value cannot decrement.
> @@ -185,13 +189,9 @@ class PRTable(object):
> return value
> else:
> return increase_revision(max_value)
> - else:
> - # "hist" mode: we found an existing value. We can return it
> - # whether it's the maximum one or not.
> - return value
>
> - def get_value(self, version, pkgarch, checksum):
> - value = self._get_value(version, pkgarch, checksum)
> + def get_value(self, version, pkgarch, checksum, history):
> + value = self._get_value(version, pkgarch, checksum, history)
> self.store_value(version, pkgarch, checksum, value)
> return value
>
> diff --git a/lib/prserv/serv.py b/lib/prserv/serv.py
> index 9e07a34445..f692d050b8 100644
> --- a/lib/prserv/serv.py
> +++ b/lib/prserv/serv.py
> @@ -76,9 +76,10 @@ class PRServerClient(bb.asyncrpc.AsyncServerConnection):
> version = request["version"]
> pkgarch = request["pkgarch"]
> checksum = request["checksum"]
> + history = request["history"]
>
> if self.upstream_client is None:
> - value = self.server.table.get_value(version, pkgarch,
> checksum)
> + value = self.server.table.get_value(version, pkgarch,
> checksum, history)
> return {"value": value}
>
> # We have an upstream server.
> @@ -105,7 +106,7 @@ class
> PRServerClient(bb.asyncrpc.AsyncServerConnection):
> # The package is not known upstream, must be a local-only
> package
> # Let's compute the PR number using the local-only method
>
> - value = self.server.table.get_value(version, pkgarch,
> checksum)
> + value = self.server.table.get_value(version, pkgarch,
> checksum, history)
> return {"value": value}
>
> # The package is known upstream, let's ask the upstream server
> --
> 2.34.1
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#16087):
> https://lists.openembedded.org/g/bitbake-devel/message/16087
> Mute This Topic: https://lists.openembedded.org/mt/105479102/1050810
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [
> bruce.ashfield@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
--
- Thou shalt not follow the NULL pointer, for chaos and madness await thee
at its end
- "Use the force Harry" - Gandalf, Star Trek II
[-- Attachment #2: Type: text/html, Size: 7993 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [bitbake-devel] [PATCH 12/12] prserv: add "history" argument to "get-pr" request
2024-04-17 15:21 ` [bitbake-devel] " Bruce Ashfield
@ 2024-04-17 15:37 ` Michael Opdenacker
0 siblings, 0 replies; 20+ messages in thread
From: Michael Opdenacker @ 2024-04-17 15:37 UTC (permalink / raw)
To: Bruce Ashfield
Cc: michael.opdenacker, bitbake-devel, Thomas Petazzoni, Joshua Watt,
Tim Orling
Hi Bruce,
Many thanks for your review, much appreciated!
On 4/17/24 at 17:21, Bruce Ashfield wrote:
> I can't say that I understand the difference between history and
> no-history from
> just reading this series .. but I realize it isn't something being
> added as part of
> this feature addition.
>
> I just wanted to comment that from the name "history" alone, I can't
> figure
> out what it is doing and how it impacts PR values.
You'll find details in the code:
https://git.openembedded.org/bitbake/tree/lib/prserv/db.py#n25
Indeed, these modes should be documented. For the moment, only the "No
history" mode was used, that's why no document exists yet.
Cheers
Michael.
--
Michael Opdenacker, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [bitbake-devel] [PATCH 00/12] prserv: add support for an "upstream" server
2024-04-12 9:02 [PATCH 00/12] prserv: add support for an "upstream" server michael.opdenacker
` (11 preceding siblings ...)
2024-04-12 9:02 ` [PATCH 12/12] prserv: add "history" argument to "get-pr" request michael.opdenacker
@ 2024-04-17 15:22 ` Bruce Ashfield
12 siblings, 0 replies; 20+ messages in thread
From: Bruce Ashfield @ 2024-04-17 15:22 UTC (permalink / raw)
To: michael.opdenacker; +Cc: bitbake-devel
[-- Attachment #1: Type: text/plain, Size: 4732 bytes --]
On Fri, Apr 12, 2024 at 5:02 AM Michael Opdenacker via
lists.openembedded.org <michael.opdenacker=
bootlin.com@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.
>
> 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.
>
>
I went through the code and commented on a few of the
larger patches.
I can't say that I'm an expert in the existing PR server and
I don't tend to write overly pythonic code .. but I did look
through things and can add:
Reviewed-by: Bruce Ashfield <bruce.ashfield@gmail.com>
> ---
>
> 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
>
>
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#16075):
> https://lists.openembedded.org/g/bitbake-devel/message/16075
> Mute This Topic: https://lists.openembedded.org/mt/105479088/1050810
> Group Owner: bitbake-devel+owner@lists.openembedded.org
> Unsubscribe: https://lists.openembedded.org/g/bitbake-devel/unsub [
> bruce.ashfield@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
>
>
--
- Thou shalt not follow the NULL pointer, for chaos and madness await thee
at its end
- "Use the force Harry" - Gandalf, Star Trek II
[-- Attachment #2: Type: text/html, Size: 7126 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread