Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Overhaul update "met requirements" checks
There are several problems in the general area of "checking if an update 'meets requirements'". Most obviously, as discussed in the comments on #3938 , there are two similar-but-different functions for this, `Update.meets_testing_requirements()` and `Update.check_requirements()`. `Update.set_request()` also sort of re-implements the same thing in-line, and there is also `Update.critpath_approved()` which does something similar. These paths all implement somewhat different requirements, which has various weird and confusing results (like the way you can sometimes push an update stable from the CLI but not from the web UI, or the way sometimes updates can be submitted stable successfully but then get ejected from the stable push for not meeting requirements). This commit introduces `meets_requirements_why`, which returns a boolean and a string explanation, and turns the existing methods into thin wrappers around it. The block in the API `set_request()` function which checked requirements is dropped because it's now redundant; the DB model `set_request()` which it calls does the same requirement check. There's no need to do it twice. The code block commented "Disable pushing critical path updates for pending releases directly to stable" does not really do that. It just applied the requirements for critical path updates on push to stable (the block immediately under it did the same for non-critpath updates). The boutique requirements to do with a "critpath.num_admin_approvals" setting were for a very old policy which gave special status to feedback from "proven testers"; that setting has been 0 in both Bodhi deployments for years and is not likely to come back, so we can just get rid of the setting and simplify the code path. This commit clarifies and simplifies the meanings of the two types of stable push threshold settings, and renames some of them. The `stable_karma` and `stable_days` properties of each update are now *only* relevant to autopush. They are never used in "can this update be pushed manually?" calculations. The `mandatory_days_in_testing` and `min_karma` settings act *only* as policy minimums for manual push eligibility. They can still be set with prefix modifiers for the release and the 'status', as before (to allow Fedora to apply different policies to different releases at different points in the cycle). If an update reaches either currently-applicable policy minimum, it is eligible for stable push. If it does not, it is not. This results in clearer logic and more consistent requirements that are more in line with the distribution policies. critpath.stable_after_days_without_negative_karma is renamed to critpath.mandatory_days_in_testing and now simply sets a days-in-testing threshold that applies to critical path updates only. The "without_negative_karma" logic is removed, because it has no basis in the Fedora or EPEL update policies; they never specified such a rule. critpath.min_karma is renamed to min_karma and applied to all updates, because this is what the current Fedora and EPEL update policies say. They both apply the same minimum karma thresholds to *all* updates. We *could* instead add min_karma and keep critpath.min_karma, but this is somewhat more work and I don't want to do more work than is actually necessary here. If the distros ever do decide to split the thresholds again, we can re-introduce critpath.min_karma. The functions and properties in the models are renamed in line with the new names of the settings, again for sanity's sake. This is of course an API change and a backwards-incompatible configuration change. As there are only two real deployments of Bodhi (both run out of the same ansible repo) and I don't think anything uses the code interfaces, this seems like it shouldn't be too much of a problem. Signed-off-by: Adam Williamson <[email protected]>
- Loading branch information
1 parent
bddee6a
commit 93598c7