search

dlang / dlang-bot

dlang-bot for automated bugzilla, github, and trello references
https://bot.dlang.io/
MIT License
22 stars 14 forks source link

Don't merge PRs where non-required status are failing #243

Closed Geod24 closed 3 years ago

Geod24 commented 4 years ago

Just happened a couple days ago, Azure was red and dlang-bot still merged a PR with auto-merge on, resulting in a few days of intermittent Azure failure. Revert is here: https://github.com/dlang/dmd/pull/11173

CyberShadow commented 3 years ago

I believe this should work as of #69, so I'm not sure what went wrong there.

There is one way that I'm aware of that this could have happened:

This does not happen with required CIs because GitHub preemptively creates "pending" statuses for the CIs that have been configured as required. With non-required CIs, this does not happen, and they are added to the list of checks only once the first status is received from the CI provider.

The only way to avoid this particular situation is to configure the appropriate CIs as required. And, actually, I don't see how things could work any other way. If the CI is not "required", but dlang-bot is expected to obey the CI status for auto-merge, then force-merging the PR by a human (thus ignoring the non-required CI failure) will have the effect of breaking the auto-merge label for everything until the failure is fixed, which is probably almost as undesirable as "breaking master" for required CIs anyway.

Hope that makes sense :)

Geod24 commented 3 years ago

There is one way that I'm aware of that this could have happened: [...]

I don't think that's what happened. From my memory, the status was already running or even already failed. I remember discussing this with Seb and it came up that it was something to do with the checks API. I set many CI as required as a result to avoid this repeating. I suppose this could be easily tested, though.

All required CI statuses pass (but no status received from Azure yet)

Due to the time each CI takes, this is extremely unlikely, if not impossible.

Since you made quite a few improvements, I'm fine with closing this and re-opening if it ever shows up again.

CyberShadow commented 3 years ago

If we were to log all GitHub hooks we receive and the responses of all API calls we do, we would have a clear picture of this should it happen again. However, the total amount of hook/API data we receive is very large, so it's impractical to log it all. Maybe with some kind of rotation and if we catch it on the same day it happens...