Re: [pbs-devel] [PATCH-SERIES v3] APT repositories API/UI

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

snipped ;)

On March 24, 2021 10:40 am, Fabian Ebner wrote:
> Am 23.03.21 um 11:29 schrieb Fabian Grünbichler:
>> the "options" part is missing support for the following "feature" ("man
>> sources.list"):
>> 
>>   Multivalue options also have -= and += as separators, which instead of
>>   replacing the default with the given value(s) modify the default
>>   value(s) to remove or include the given values.
>> 
>> I haven't tested that one though ;)
>> 
> 
> The parser just uses "option+" and "option-" as the option keys then. It 
> /could/ be interpreted on parsing, but if the user chose to use this 
> notation, there's probably a reason and it should be written out the 
> same way it came in again. I'll add a test though.

yeah, makes sense (in a way).

> 
>> if a file cannot be parsed or is malformed (e.g. because I put "Uris"
>> instead of "URIs" in a .sources file ;)), the whole API call fails with
>> 400. it might be more user-friendly to mark indiviual .list/.sources
>> files as containing invalid entries which are not displayed, and still
>> return the rest? might make the result less actionable since we don't
>> have the complete picture, but it still might be better than a single
>> error message for one of X files..
>> 
> 
> I did consider something like this for a bit, but not sure how to 
> cleanly organize the API then. The call should still error out in my 
> opinion and syntactic errors should be rather rare anyways (apt also 
> just complains when it cannot parse). We could continue parsing and 
> collect all the errors at once at least, but not sure if that's worth it?

I was thinking more of further inconsistencies between APT and our 
parser like the case-sensitivity issue. like you said, APT will just 
ignore a wrong/broken entry or file, and not error out altogether. so 
maybe it would make sense to mimic that behaviour -> add a 
warnings/error field, and let the caller decide whether it just wants to 
display that or the (partial) result + the additional information. e.g., 
we could disable editing via the GUI for invalid files, but still allow 
it for other files.

IMHO for this the request as a whole does not have to error out on the 
API/HTTP level if the parser encounters something it does not 
understand.

>> we have a warning for Debian unstable, but none for Debian testing which
>> should also never be enabled on a production machine.
>> 
> 
> If there is an "upgrade suites" button/API call, then there would be 
> warnings after using that. But since enabling that button/API call needs 
> to happen anyways before each major release, I guess removing the e.g. 
> 'bullseye' warnings then is just one more place to touch.

no, because 'bullseye' is not 'testing' ;) or at least, they should not 
be treated the same. having 'bullseye' as a suite is fine when upgrading 
from 'buster' to 'bullseye'. having 'testing' there is never good on a 
stable/production system.

> Or maybe add a 'before_major_release' parameter to check_repositories 
> and also to the UI? Then only the product specific code needs to be 
> touched before each major release. Of course there still needs to be a 
> new version of the library for after each major release.

the check for 'bullseye' could contain text that indicates that the 
warning is benign if you are preparing the major release upgrade? or the 
'upgrade suites' button sets a flag somewhere (comment? ;)), and that 
then automatically skips that check for the given suite?

>> we might want to match known-official repo URIs (ftp.*.debian.org,
>> deb.debian.org, download.proxmox.com, enterprise.proxmox.com) to mark
>> "potentially dangerous external repositories" (or to give the official
>> ones a "official mirror" badge or something like that)
>> 
> 
> Might be done as part of the check_repositories call if we go for 
> warnings. If we go for badges, we'd need something new or change the 
> interface for check_repositories. I feel like badges would be preferable 
> though...

badges are nicer since they just highlight "this is good", which is 
easier to determine than "this is bad" (which is actually just "this 
might be bad"). having some custom repo that is okay is not that 
uncommon (internal software, monitoring, ansible integration packages, 
..), so being too noisy might be a bad idea.