bodhi claims "can be pushed to stable" when stable_karma - 1 is reached

[decathorpe]

For example:

https://bodhi.fedoraproject.org/updates/FEDORA-2019-886409cae5

This update got +2 karma, but needs +3. Bodhi still just sent me a notification that I can push it to stable (which I can't, since it needs +3 karma, not +2).

[AdamSaleh]

Based on e17252f it seems this works as intended? At least by the looking at the code, anytime you have more than critpath_min_karma it assumes testing is finished and you can go forward. @bowlofeggs do I read that correct?

[decathorpe]

Even if that's true, the message is still wrong.

The minimum karma for pushing an update to stable is a value that can be set for each update individually - +3 by default (+1 is the minimum for normal updates, and +2 is the the minimum value for critpath packages).

It makes no sense to show "this can be pushed to stable now" when the minimum positive karma for that update hasn't been reached yet.

[karmadolkar]

Probably there's an error in where we send that message. I'd like to work on this issue @cverna

[cverna]

@karmadolkar this is where the comment is made --> https://github.com/fedora-infra/bodhi/blob/develop/bodhi/server/tasks/approve_testing.py#L81

I would look at how the meets_testing_requirements methods works here --> https://github.com/fedora-infra/bodhi/blob/develop/bodhi/server/models.py#L3674

[cverna]

So after looking at this with @karmadolkar it seems that a lot of code assumes that when a critpath update reaches the critpath_min_karma (configured to 2) the update can be pushed to stable, even if the packager set the update with an higher karma number. That's seems wrong to me but befora changing that I prefer to double check it.

I think that a critpath update should not be able to have a positive karma threshold lower than critpath_min_karma so if you try to set that threshold to 1 and critpath_min_karma is 2, bodhi will let you know that 1 is not valid and replace it with 2. But if the packager set the positive karma threshold to 3 then that value should be used instead of the critpath_min_karma.

@AdamWill @kparal does that ^^ makes sense ? Or is there a special case for critpath updates ?

[AdamWill]

"So after looking at this with @karmadolkar it seems that a lot of code assumes that when a critpath update reaches the critpath_min_karma (configured to 2) the update can be pushed to stable, even if the packager set the update with an higher karma number. That's seems wrong to me but befora changing that I prefer to double check it."

This is sort of tricky, because it depends on what you believe the packager-set threshold is for. According to the updates policy, any critpath update can be manually pushed stable at +2, and any other update can be manually pushed stable at +1. Certainly at first, in Bodhi the karma threshold was specifically intended as a threshold for automatic push, and it was entirely intentional that the update owner (or any other proven packager) could still push the update stable manually if it had not yet reached this threshold, so long as it reached the policy-defined minimum.

We do actually rely on this still, quite a lot, for pushing blocker / FE fixes stable. If we had to get them all to the autopush threshold (which still tends to be left at +3 for almost all updates) before we could push them, that would be a pain.

So, I definitely agree with "I think that a critpath update should not be able to have a positive karma threshold lower than critpath_min_karma so if you try to set that threshold to 1 and critpath_min_karma is 2, bodhi will let you know that 1 is not valid and replace it with 2.", but the other part I'm not so sure about. Note that the UI description for the threshold still matches the reading I gave above. It's called "Auto-request stable based on karma?" and described as "If checked, this option allows bodhi to automatically move your update from testing to stable once enough positive karma has been given." - i.e. it's clear that the threshold is for automatic push, there's no suggestion in the name or description that this is the minimum value for it to be pushed stable in any way at all.

[cverna]

Ok thanks for the explanation that makes things easier.

I think the current way bodhi does that is correct, even the message that the update can be pushed to stable when the karma reached 2 even if the packager set the automatic stable push karma to 3 is then valid.
But we need to make sure that the the packager can then push that update to stable, I have not looked at this in details but from the bug report from @decathorpe it seems that it was not possible to push the update.

[decathorpe]

@cverna I think it would have been possible if I edited the update and reduced the stable karma to +2, but I would need to look at another case of this to verify it.

If I understand correctly, the built-in +1/+2 is only the lower bound for karma that allows pushing to stable, and the user-set karma limit (default to +3) decides about automatic karma-based push.

So the message about "this can be pushed to stable" at karma +2 is only a bit misleading, it should say "this has reached the minimum amount of karma to reach stable, but has not reached the positive karma threshold yet"

[AdamWill]

You shouldn't need to edit the autopush threshold in order to push manually.

I actually tend to do manual pushes by CLI, because the CLI seems to get the rules right whereas I agree the web UI does not always seem to, it sometimes does not allow manual push when it should. CLI behaviour always seems to be correct.

[AdamWill]

@decathorpe I don't think your edit is any better, to be honest. I'd write something like "This update can now be manually submitted for stable, but will not be automatically submitted for stable unless it reaches the automatic threshold which is currently set to X"?

[decathorpe]

@AdamWill Sure, it was just a suggestion. Yours is better, but the karma threshold isn't necessarily set to +3 automatically, so I'd drop that qualifier as well. Then I'm okay with it :) And you're right, the web UI should definitely allow pushing to stable once the requirements are satisfied

Right now it's more like, "you can push this to stable now, if you want to, but we won't show you the button to do it"

(pending verification that the button actually is missing in this case)

[AdamWill]

that's why I wrote X. The message can fairly trivially read the current value...

[karmadolkar]

@cverna and I tested to see if the webpage displays the "push to stable" button when +2 karma is reached. We found it to be behaving correctly.