Closed Bug 1825961 Opened 1 year ago Closed 6 months ago

Missing uplift approval flags on uplift patches causes BugBot to falsely complain

Categories

(bugzilla.mozilla.org :: Phabricator Integration, defect)

defect

Tracking

()

RESOLVED FIXED

People

(Reporter: suhaib, Assigned: sheehan)

References

Details

Attachments

(3 files)

Uplift patches created using the uplift workflow through Lando do not have approval flags (i.e., approval-mozilla-*). When such a patch is attached to a closed bug, autonag will consider it as a new patch, which triggers the bot to suggest opening a new bug (see mozilla/relman-auto-nag#1953).

Any of the following solutions could resolve the issue:

  1. Keep backward compatibility with the current workflow by syncing the approval status with Bugzilla. So adding an approve request flag when attaching the uplift patch to the bug (e.g., approval-mozilla-beta?) and changing it to approved (e.g., approval-mozilla-beta+) when the patch lands or get approved on Phapricator.

  2. Add a flag to identify the attachment as an uplift patch without syncing the approval status.

Assignee: nobody → sheehan

After the patches above are deployed, the approval-mozilla-{repo} flags will be added to new bug attachments for Phabricator revisions on uplift patches and the flag status will be synced with the Phabricator revision status.

Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED

The new uplift workflow appears to be failing to add approval flags to the uplift patches.

Example: https://bugzilla.mozilla.org/show_bug.cgi?id=1842547#c13

:sheehan could you please investigate the issue?

Status: RESOLVED → REOPENED
Flags: needinfo?(sheehan)
Resolution: FIXED → ---
Summary: Missing uplift approval flags on uplift patches causes autonag to falsely complain → Missing uplift approval flags on uplift patches causes BugBot to falsely complain

It seems to be intermittent, or at least not impacting every patch. You can see from this example that the approval flag is sometimes being set.

The example I've given shows the ESR flags being set, perhaps it's only beta which is broken. I'll need to investigate further.

See Also: → 1844183

This should hopefully be fixed now with recent changes. Please reopen if this occurs again.

Status: REOPENED → RESOLVED
Closed: 1 year ago10 months ago
Flags: needinfo?(sheehan)
Resolution: --- → FIXED

This is not working for beta, only for esr. Relman is manually setting these to approval-mozilla-beta+

Example of it working in esr115
Example of it NOT working in beta

Please let me know if I should I file a new bug.

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

Hey Dianna, thanks for letting me know about this. Any idea how long the flag hasn't been set automatically for beta? Is this a recent change or has this been going on for a while?

Flags: needinfo?(dsmith)

It wasn't working in the 120 beta cycle but I do not know for how long . I remember it did it once, but it must've been esr.

Flags: needinfo?(dsmith)

(In reply to Dianna Smith [:diannaS] from comment #8)

This is not working for beta, only for esr. Relman is manually setting these to approval-mozilla-beta+

Example of it NOT working in beta

Please let me know if I should I file a new bug.

For 1866479, it seems the approval-beta flag was not set as chutten (the revisions author) is not a member of the mozilla-next-drivers group. That group is needed to set '+' on the beta flag. We can add him to the group to fix this for any future revisions authored by him, but I am sure someone else will run into this issue. I am sure we don't want to open up to everyone to set the approval-beta flag so maybe one solution would be to use a different account with required permissions to set the approval-beta flags. Such as lando-bot or phab-bot maybe.

(In reply to David Lawrence [:dkl] from comment #11)

For 1866479, it seems the approval-beta flag was not set as chutten (the revisions author) is not a member of the mozilla-next-drivers group. That group is needed to set '+' on the beta flag. We can add him to the group to fix this for any future revisions authored by him, but I am sure someone else will run into this issue. I am sure we don't want to open up to everyone to set the approval-beta flag so maybe one solution would be to use a different account with required permissions to set the approval-beta flags. Such as lando-bot or phab-bot maybe.

I think I understand the problem now. When the revision is created, we want the revision's author to be the user who sets the approval-mozilla-beta? flag on the Bugzilla attachment. However when the status of the patch goes to "Accepted", we want the release manager (the reviewer of the patch, not the submitter) to be the one who sets the flag to approval-mozilla-beta+. If the user who accepts the patch can't set approval-mozilla-beta+ due to their lack of membership in the mozilla-next-drivers group, the flag's status should not change. This should be the case for approval-mozilla-esr115+ etc as well.

ty! this actually makes sense, because the author of tat esr flag was ryan. It is possible the instances I previously saw were all ones I created myself.

(In reply to Connor Sheehan [:sheehan] from comment #12)

(In reply to David Lawrence [:dkl] from comment #11)
I think I understand the problem now. When the revision is created, we want the revision's author to be the user who sets the approval-mozilla-beta? flag on the Bugzilla attachment. However when the status of the patch goes to "Accepted", we want the release manager (the reviewer of the patch, not the submitter) to be the one who sets the flag to approval-mozilla-beta+. If the user who accepts the patch can't set approval-mozilla-beta+ due to their lack of membership in the mozilla-next-drivers group, the flag's status should not change. This should be the case for approval-mozilla-esr115+ etc as well.

That should be simple enough to change as would just use the bmo account of the revision changer instead of the revision author which we also know at the time.

Status: REOPENED → RESOLVED
Closed: 10 months ago6 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: