Skip to content
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.

Sign up for GitHub

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

Fix rebase API race condition #173

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix rebase API race condition #173

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 9 additions & 23 deletions marge/job.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,12 @@ def update_from_target_branch_and_push(
final_sha = self.add_trailers(merge_request) or updated_sha
commits_rewrite_done = True
branch_was_modified = final_sha != initial_mr_sha
self.synchronize_mr_with_local_changes(merge_request, branch_was_modified, source_repo_url)

if self._options.fusion is Fusion.gitlab_rebase:
final_sha = self.synchronize_using_gitlab_rebase(merge_request)
else:
self.push_force_to_mr(merge_request, branch_was_modified, source_repo_url)

except git.GitError:
if not branch_update_done:
raise CannotMerge('got conflicts while rebasing, your problem now...')
Expand All @@ -306,21 +311,6 @@ def update_from_target_branch_and_push(
repo.checkout_branch('master')
repo.remove_branch(source_branch)

def synchronize_mr_with_local_changes(
self,
merge_request,
branch_was_modified,
source_repo_url=None,
):
if self._options.fusion is Fusion.gitlab_rebase:
self.synchronize_using_gitlab_rebase(merge_request)
else:
self.push_force_to_mr(
merge_request,
branch_was_modified,
source_repo_url=source_repo_url,
)

def push_force_to_mr(
self,
merge_request,
Expand All @@ -347,8 +337,8 @@ def fetch_remote_branch():
change_type = "merged" if self.opts.fusion == Fusion.merge else "rebased"
raise CannotMerge('Failed to push %s changes, check my logs!' % change_type)

def synchronize_using_gitlab_rebase(self, merge_request, expected_sha=None):
expected_sha = expected_sha or self._repo.get_commit_hash()
def synchronize_using_gitlab_rebase(self, merge_request):
"""Return the SHA of the MR after GitLab has rebased it."""
try:
merge_request.rebase()
except MergeRequestRebaseFailed as err:
Expand All @@ -365,11 +355,7 @@ def synchronize_using_gitlab_rebase(self, merge_request, expected_sha=None):
raise CannotMerge("Sorry, I can't modify protected branches!")
raise
else:
if merge_request.sha != expected_sha:
raise GitLabRebaseResultMismatch(
gitlab_sha=merge_request.sha,
expected_sha=expected_sha,
)
return merge_request.sha


def _get_reviewer_names_and_emails(commits, approvals, api):
Expand Down
9 changes: 5 additions & 4 deletions marge/merge_request.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,18 +138,19 @@ def rebase(self):
# We wanted to rebase and someone just happened to press the button for us!
log.info('A rebase was already in progress on the merge request!')

max_attempts = 30
wait_between_attempts_in_secs = 1
# Wait for at most 2 minutes until giving up.
max_attempts = 24
wait_between_attempts_in_secs = 5

for _ in range(max_attempts):
time.sleep(wait_between_attempts_in_secs)

self.refetch_info()
if not self.rebase_in_progress:
if self.merge_error:
raise MergeRequestRebaseFailed(self.merge_error)
return

time.sleep(wait_between_attempts_in_secs)

raise TimeoutError('Waiting for merge request to be rebased by GitLab')

def accept(self, remove_branch=False, sha=None):
Expand Down