fedora-infra / bodhi

Bodhi is a web-system that facilitates the process of publishing updates for a Fedora-based software distribution.
https://bodhi.fedoraproject.org
GNU General Public License v2.0
151 stars 190 forks source link

Consider whether to remove the critpath_karma field #2194

Open bowlofeggs opened 6 years ago

bowlofeggs commented 6 years ago

Bodhi has a critpath_karma field that does not seem to be used for anything other than display in the UI. I started a discussion to find out about its history. Perhaps we can remove it to clean some code up and to make the UI less confusing.

This field is certainly confusing to users. I've observed users giving critical path karma instead of normal karma in updates, which can lead to bad results. For example, three users gave this update critpath karma -1's, which were not recorded as karma on the update and thus would not stop it from going to stable automatically. IMO, the critpath karma field is harmful in this example.

If we do decide to remove it, consider whether it makes sense to keep the data in the database for history, or to drop it due to it not being useful.

keszybz commented 6 years ago

+1 to removing it.

AdamWill commented 6 years ago

I explained why this exists and what it was meant for here. I think it might be nice to actually revisit the ideas which this (and other currently under-used things in Bodhi) was meant to support, rather than just taking it out.

bowlofeggs commented 6 years ago

Cool. I like those ideas.

I propose that we do those ideas (in a separate ticket, yet to be filed) and use this ticket to make a simple change: if a user gives a -1 to critpath karma, automatically consider their regular karma to also be -1 (no matter what they set it to?). How does that sound?

ferdnyc commented 6 years ago

Some of that was rolled into the broader karma-handling, no? I'm thinking of this one, specifically:

  1. Any update that is marked as 'critpath breaking' by a FAS-registered tester would be blocked from going any further in the update process without manual intervention (no autopushes at all)

As things now operate, Bodhi seems to disable autopush on receipt of negative regular karma, it isn't limited to the critpath karma.

(Which is probably the safer response. The concept of negative critpath karma as Bodhi's "big red Panic! button" for an update is an intriguing one. But because hitting it is intended to be such a Big Deal™, cautious automated responses like disabling autopush probably shouldn't be reserved for only instances where someone hits it. The next proposal item,

  1. Any update marked as 'critpath breaking' by a proven tester would be blocked from being pushed stable at all - automatically or manually - until [mitigations]

defines an appropriately escalated autoresponse to the critpath Panic! button.)

keszybz commented 6 years ago

This is outside of the scope of the initial proposal, but since we're discussing various other ideas, I'll throw this out here as well:

I think we should also get rid of the negative karma threshold. IIUC, any negative karma currently disables autopush. Thus the negative karma threshold in bodhi is not useful, and should be removed.

I explained why this exists and what it was meant for [here].

We don't have proventesters any more, no? I'm not convinced to we need automatism to stop pushes. If a tester leaves a comment like "OMG, this update breaks critpath, when ...", and -1 karma, the update will not be pushed automatically. If the maintainer pushes the update to stable despite that feedback, the problem is with the maintainer. I trust our maintainers not to do this.

I still think we should remove the critpath karma from UI. If there are plans for the future, it might be better to keep the backend code, inactive, rather than removing it outright.

AdamWill commented 5 years ago

"I think we should also get rid of the negative karma threshold. IIUC, any negative karma currently disables autopush. Thus the negative karma threshold in bodhi is not useful, and should be removed."

This is not all it does, though. -3 karma automatically unpushes the update. This is actually quite important and useful, as it means when a really bad update gets submitted, a flood of negative karma can get it removed from updates-testing without the update owner or a provenpackager having to step in and manually unpush it.

ryanlerch commented 4 years ago

I still think we should remove the critpath karma from UI. If there are plans for the future, it might be better to keep the backend code, inactive, rather than removing it outright.

I think this is probably the best route at the moment. the UI was recently updated in develop, so removing it is relatively easy.

Additionally, as per #2183, the cli doenst even have the ability to change critpath karma, so its even easier to just remove it from the webUI only.

mattiaverga commented 4 years ago

This was closed by a PR, but it's not entirely fixed: the critpath_karma field and logic are still there, so I'm going to reopen this issue.

ferdnyc commented 4 years ago

@mattiaverga: In the PR @ryanlerch cited https://github.com/fedora-infra/bodhi/issues/2194#issuecomment-370977735 above as the reason not to remove the backend parts, since @AdamWill wrote:

I think it might be nice to actually revisit the ideas which this (and other currently under-used things in Bodhi) was meant to support, rather than just taking it out.

AdamWill commented 4 years ago

I mean, I do still think that, but on the other hand the chances of me having time to get back around to it any time soon are slim to none, and we always have git history, so I'm still not gonna cry if it's taken out.