| Message ID | 20240412090234.4110915-13-michael.opdenacker@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | prserv: add support for an "upstream" server | expand |
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] > -=-=-=-=-=-=-=-=-=-=-=- > >
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.
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