search

mozilla / bugbot

A Mozilla release management tool to send reminders to Firefox developers and improve Bugzilla metadata
BSD 3-Clause "New" or "Revised" License
36 stars 66 forks source link

Handle failures in needinfo requests #1370

Open suhaibmujahid opened 2 years ago

suhaibmujahid commented 2 years ago

Example of requesting needinfo from a disabled account:

2022-03-28 12:10:40,416 - ERROR - not_landed: Cannot put data for bug 1651425 (change => {'comment': {'body': 'There\'s a r+ patch which didn\'t land and no activity in this bug for 2 weeks.\n:bpeers, could you have a look please?\nIf you still have some work to do, you can add an action "Plan Changes" in Phabricator.\nFor more information, please visit [auto_nag documentation](https://wiki.mozilla.org/Release_Management/autonag#not_landed.py).\n'}, 'flags': [{'name': 'needinfo', 'requestee': '[bpeers@mozilla.com](mailto:bpeers@mozilla.com)', 'status': '?', 'new': 'true'}, {'name': 'needinfo', 'requestee': '[snorp@snorp.net](mailto:snorp@snorp.net)', 'status': '?', 'new': 'true'}]}).

From Discussion in the Matrix Room

@marco-c

we have the same problem in all tools that are needinfoing maybe we should implement something generic try to needinfo and, if it fails, needinfo their manager instead (with an option so individual tools can disable it)

@calixteman

the triage owner could make more sense but yes we must have something here

@marco-c

triage owner wfm

@calixteman

I think we don't have to do it for workflow tools (they are only for mozilla employees and we already have a fallback strategy iirc) for the others we can just add a method fallbackInCaseOfError or smthg like that we could call in the tool to "fix" the error it will update the data to use and then the loop in bzcleaner will continue to PUT after the update

@suhaibmujahid

we could do something generic in bzcleaner with an option in the tools themselves to select the strategy, e.g., fallback to the triage owner or the manager or try both in some order getting the manager if an employee left is a separate issue (see #1354) and resolving it can help here also

marco-c commented 2 years ago

It'd be simple to at least add something here (https://github.com/mozilla/relman-auto-nag/blob/ea734c75f736c1094f6249e9a942b4cf87ace99f/auto_nag/bzcleaner.py#L508) to skip needinfo on inactive people (using the UserActivity class). The problem is that in some cases we both do an autofix change and set a needinfo -> instead, if we can't do the needinfo we shouldn't do the autofix change either.