On Sun, 2024-11-17 at 18:35 +0100, Kevin Kofler via devel wrote: > Kevin Kofler via devel wrote: > > PS: I suspect the documentation was actually just an attempt to document > > the previous broken Bodhi policy implementation. (See > > https://github.com/fedora-infra/bodhi/issues/772 and > > https://github.com/fedora-infra/bodhi/issues/1033 – both got closed, but > > the issue was never properly fixed.) > > PPS: Sorry, one more thing, I have to correct myself: > > Actually, the issue was eventually fixed in that Bodhi has at some point > started allowing to set the stable threshold even when unchecking the > autopush checkbox, so it was possible before 8.2 to manually push updates to > stable at +1. > Though I still do not understand how a threshold makes any sense at all for > manual pushes. Manual pushes should just always require only the minimum > required by policy, as the person setting the threshold (to any allowed > value) is the same as the one doing the push. > > Settable thresholds have only ever made sense for automatic pushes. (So the > bug was not that the threshold was not settable for manual pushes, but that > the non-settable threshold was fixed to the wrong value, the default for > automatic pushes (+3) instead of the minimum allowed (+1). Which is why I > still do not consider this properly fixed.) Yes, to be clear, in rewriting this code, I had the same opinion as you. The configurable thresholds only make sense as applying to autopush. There is no reason I can see to consider them as applying to manual push. To me, as to you, the only constraint on manual push that makes sense is a policy minimum karma for manual pushes, which naturally cannot be edited. As rewritten, the minimum requirements for manual push are different concepts from the autopush thresholds and are now more clearly defined as such in Bodhi's code. The per-update editable thresholds now clearly (in the code) have absolutely no input into the decision as to whether an update can be manually pushed stable. Whatever those thresholds are set to, if the update meets the currently-configured policy minimum requirements for stable push (and any gating requirements), it can be pushed; if not, it can't. You can verify that in the code, if you like. So I think your 'issue' as you describe it above is addressed in 8.2. The situation in the old code was actually rather more complicated, and weird, than "the non-settable threshold was fixed to the wrong value" - there were multiple check functions that applied different logic to different operations, which is why you could sometimes push something via the CLI that you could not push via the web UI, for e.g. > Still, somehow a +2 threshold made its way into the documentation, and I am > still wondering on what base that was encoded there. That is in fact an interesting question, now I dig into it. Zbigniew seems to have done substantial rewrites on the karma requirements in 2021. These were: https://pagure.io/fesco/fesco-docs/c/e57f002ee55810d1636e4460fe0d2c3515897c0b?branch=main https://pagure.io/fesco/fesco-docs/c/19bf26cef5d5e2a8d744ea28d8d54ec5374112e9?branch=main https://pagure.io/fesco/fesco-docs/c/33c510e462f0b4118a716a89315f6afef6f5354c?branch=main The "some more" in the first commit description refers to 997bd212d760f223ecaa98cbc5e020ad35754b01 by Kevin, which made substantial updates to the policy but did not change the karma stuff AFAICS. As of that commit, these were the requirements: u-t activation to Beta release: critpath: "*MUST* either get one +1 karma *OR* spend at least 14 days in updates-testing and have no negative karma" non-critpath: "*MUST* either reach the prescribed karma level for that update *OR* spend at least 3 days in updates-testing" Beta to Final: critpath: "*MUST* either have a sum of +2 karma *OR* spend at least 14 days in updates-testing and have no negative karma" non-critpath: "*MUST* either reach the prescribed karma level for that update *OR* spend at least 7 days in updates-testing" After Final: critpath: "needs to have either a Bodhi karma sum of 2 *OR* It must spend at least 14 days in updates-testing *AND* have no negative Bodhi karma points" non-critpath: "*MUST* either [reach the critpath requirements] *OR* reach the positive Bodhi karma threshold specified by the updates submitter *OR* spend some minimum amount of time in updates-testing, currently one week" The "after Final" and "Beta to Final" requirements are effectively the same, just written differently. At this point, we don't actually have any explicit minimum karma requirement for non-critpath updates; it's phrased as "reach the prescribed karma level for that update", which you and I have both noted doesn't really make any sense, and is dependent on the behavior of the tool. *Effectively*, given Bodhi's behavior at this point, this more or less meant the minimum was 1. After Zbigniew's first commit, they looked like this: "* The update becomes eligible for being pushed manually after reaching the minimum "Stable by Karma" threshold OR the minimum "Stable by Time" threshold. The maintainer is free to set the thresholds, but they cannot be lower than the minimum values described below, enforced by Bodhi." "=== Non-critical path update thresholds Stable by Karma: minimum +1, default +3 + Unstable by Karma: maximum -1, default -3 + Stable by Time (days): minimum 3, default 3, between <<updates-testing- activation>> and <<beta-freeze>> + minimum 7, default 7, after <<beta-freeze>> === Critical path and EPEL update thresholds Stable by Karma: minimum +2, default +3 + Unstable by Karma: maximum -1, default -3 + Stable by Time (days): minimum 3, default 3, between <<updates-testing- activation>> and <<beta-freeze>> + minimum 14, default 14, after <<beta-freeze>>" Note that we now have an explicit minimum for non-critpath (though defined in a somewhat odd way), and the "no negative karma" requirement for critpath has disappeared. After his second commit, the minimum definitions were changed to this: "=== Non-critical path update thresholds Stable by Karma: minimum +1, default +3, between <<updates-testing- activation>> and <<beta-freeze>> + minimum +2, default +3, after <<beta-freeze>> + Unstable by Karma: maximum -1, default -3 + Stable by Time (days): minimum 3, default 3, between <<updates-testing- activation>> and <<beta-freeze>> + minimum 7, default 7, after <<beta-freeze>> === Critical path and EPEL update thresholds Stable by Karma: minimum +1, default +3, between <<updates-testing- activation>> and <<beta-freeze>> + minimum +2, default +3, after <<beta-freeze>> + Unstable by Karma: maximum -1, default -3 + Stable by Time (days): minimum 3, default 3, between <<updates-testing- activation>> and <<beta-freeze>> + minimum 14, default 14, after <<beta-freeze>>" *This* is the point at which a minimum of +2 was introduced for non- critpath updates (after Beta freeze). The commit message does not reference any FESCo decision. It says "This matches what bodhi seems to implement and the text before my changes said", but I don't believe either of those things is accurate: it does not really match what the text before his changes said (it did not clearly prescribe +2 for non- critpath), and it does not really match old Bodhi behaviour (which was weird and inconsistent, but *could* be made to push a non-critpath update with only +1 karma). The third commit just tweaked the preamble text a bit, it didn't functionally change any of the requirements. So on the whole, I think you have a good point here, thanks for raising it. I will file a FESCo ticket and ask for them to consider the history here and decide if they want to make any changes based on this evaluation. I think the current policy is written in a much more sensible and clearer form than the old one was, but the actual changes to the requirements that happened along the way may have been unintentional. -- Adam Williamson (he/him/his) Fedora QA Fedora Chat: @adamwill:fedora.im | Mastodon: @adamw@xxxxxxxxxxxxx https://www.happyassassin.net -- _______________________________________________ devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
