-
Notifications
You must be signed in to change notification settings - Fork 136
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
Recognise 'Branch cannot be merged' error and retry later #292
base: master
Are you sure you want to change the base?
Conversation
4bf0f35
to
903dd5e
Compare
Maybe! Another option might be to call this function which implements a simple linear backoff: https://github.com/smarkets/marge-bot/pull/265/files -- BTW do you know why this error occurs? |
Thanks for your reply! I was actually working on an outdated version (just recently rebased on latest version) when I originally made this commit internally a while ago. The commit you pointed out might be making this PR redundant. When I investigated it in the past, it was happening because while marge was trying to merge, another MR was merged in manually by a user, which required the MR marge was merging to be rebased / updated (per our gitlab configuration) before it could be merged. If you expect your change to fix this issue I can report back if I ever get this issue again (now with v0.9.5). |
That's interesting @nirizr - I'm not sure whether my PR would fix this or not. My changes assume that the MR is good to merge, but that the GitLab API endpoint isn't yet returning the correct result. In your situation, you need Marge to realise that the MR now cannot be merged, and then rebase again. I'd be interested to find out whether you still experience this bug with version 0.9.5, but my suspicion is that you will. |
If you don't believe your fix applies here. What could be the risk of merging it in? Would you prefer waiting until I can confirm its still helpful? I haven't seen that behavior with v0.9.5 but we're more frequently using Marge now (compared to manual merging we used to do a lot when I just brought Marge in). |
Hi @nirizr I'm not a maintainer of this project, so I can't help with merging your change. I think it would be useful to confirm that the bug still occurs in v0.9.5, but that's just my opinion! |
Yea, I was asking for your opinion :) |
Ha! Well, personally I'm always cautious about merging, but maybe that's just me ;) |
Hi, wondering if this is still needed for v0.9.5+? |
Hi @qqshfox, thanks for taking the time! I believe I've encountered the error still with latest marge version. Keep in mind, though, that company I work for was recently bought so we no longer use Marge bot. |
Interesting. We've been using v0.9.5 for 2+ months at Smarkets, and not seen this again. I'll leave this open in case anyone encounters this issue with the latest version again. |
We encountered it because users were also merging (and pushing to master, btw) manually. Do you do that at Smarkets? LMK if there's anything else I can maybe help with. |
No. We don't allow pushing to master. But wait, is that the expected behavior to give up merging when there are new commits in master? |
Our gitlab configuration was such that only fast forward merges were allowed (so users would often rebase on top of master just before merging, to run our pipeline on the most up to date code) into master. From memory, what would happen was this:
|
I've seen the 'Branch cannot be merged' several times due to a race condition between margebot and pipelines been rerun.
If this occurs, IMHO margebot shouldn't necessarily give up immediately but instead retry again which is achieved by keeping marge bot the assignee.
WDYT?