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

[Michael Opdenacker <michael.opdenacker@bootlin.com>]

Hi Richard,

Many thanks for the first review!

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


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

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


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

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


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

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