Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider whether to remove the critpath_karma field #2194

Closed
bowlofeggs opened this issue Mar 6, 2018 · 7 comments · Fixed by #3521

Comments

@bowlofeggs
Copy link
Member

@bowlofeggs bowlofeggs commented Mar 6, 2018

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

This comment has been minimized.

Copy link

@keszybz keszybz commented Mar 6, 2018

+1 to removing it.

@AdamWill

This comment has been minimized.

Copy link
Contributor

@AdamWill AdamWill commented Mar 7, 2018

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

This comment has been minimized.

Copy link
Member Author

@bowlofeggs bowlofeggs commented Mar 7, 2018

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

This comment has been minimized.

Copy link

@ferdnyc ferdnyc commented Apr 8, 2018

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

This comment has been minimized.

Copy link

@keszybz keszybz commented Apr 9, 2018

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

This comment has been minimized.

Copy link
Contributor

@AdamWill AdamWill commented Dec 12, 2018

"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.

@bowlofeggs bowlofeggs removed this from To do in Release Bodhi 4.0.0 Mar 22, 2019
@ryanlerch

This comment has been minimized.

Copy link
Contributor

@ryanlerch ryanlerch commented Sep 26, 2019

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.

ryanlerch added a commit to ryanlerch/bodhi that referenced this issue Sep 26, 2019
for a long time, there has been a critpath_karma field
in an updates comments that users could use on critical path
updates. However, it was not used by bodhi to do anything.

This PR removes the critpath_karma from the ui, but leaves
everything else for now. see fedora-infra#2194 for more information.

Additionally, the bodhi cli never had support for changing
the critpath_karma.

Fixes: fedora-infra#2194
Fixes: fedora-infra#2183

Signed-off-by: Ryan Lerch <rlerch@redhat.com>
ryanlerch added a commit to ryanlerch/bodhi that referenced this issue Sep 26, 2019
for a long time, there has been a critpath_karma field
in an updates comments that users could use on critical path
updates. However, it was not used by bodhi to do anything.

This PR removes the critpath_karma from the ui, but leaves
everything else for now. see fedora-infra#2194 for more information.

Additionally, the bodhi cli never had support for changing
the critpath_karma.

Fixes: fedora-infra#2194
Fixes: fedora-infra#2183

Signed-off-by: Ryan Lerch <rlerch@redhat.com>
@mergify mergify bot closed this in #3521 Sep 26, 2019
mergify bot added a commit that referenced this issue Sep 26, 2019
for a long time, there has been a critpath_karma field
in an updates comments that users could use on critical path
updates. However, it was not used by bodhi to do anything.

This PR removes the critpath_karma from the ui, but leaves
everything else for now. see #2194 for more information.

Additionally, the bodhi cli never had support for changing
the critpath_karma.

Fixes: #2194
Fixes: #2183

Signed-off-by: Ryan Lerch <rlerch@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.