Skip to content

Commit

Permalink
Use target branch name in batch merge to allow multiple instances
Browse files Browse the repository at this point in the history
A project can use multiple instances of marge bots assigned to
different target branches. This allows dealing with different
release branches in parallel.

However batches could not be used on multiple instances because
they conflicted on the name of the branch created for the batch.
By naming this branch after the target branch, then multiple
batch branches can exist.
  • Loading branch information
valentindavid committed Aug 31, 2020
1 parent ca64531 commit 0585293
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 17 deletions.
26 changes: 15 additions & 11 deletions marge/batch_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,17 @@ def __init__(self, *, api, user, project, repo, options, merge_requests):
super().__init__(api=api, user=user, project=project, repo=repo, options=options)
self._merge_requests = merge_requests

def remove_batch_branch(self):
def remove_batch_branch(self, target_branch):
log.info('Removing local batch branch')
try:
self._repo.remove_branch(BatchMergeJob.BATCH_BRANCH_NAME)
self._repo.remove_branch(f'{BatchMergeJob.BATCH_BRANCH_NAME}/{target_branch}')
except git.GitError:
pass

def close_batch_mr(self):
def close_batch_mr(self, target_branch):
log.info('Closing batch MRs')
params = {
'target_branch': target_branch,
'author_id': self._user.id,
'labels': BatchMergeJob.BATCH_BRANCH_NAME,
'state': 'opened',
Expand All @@ -48,7 +49,7 @@ def close_batch_mr(self):
def create_batch_mr(self, target_branch):
log.info('Creating batch MR')
params = {
'source_branch': BatchMergeJob.BATCH_BRANCH_NAME,
'source_branch': f'{BatchMergeJob.BATCH_BRANCH_NAME}/{target_branch}',
'target_branch': target_branch,
'title': 'Marge Bot Batch MR - DO NOT TOUCH',
'labels': BatchMergeJob.BATCH_BRANCH_NAME,
Expand Down Expand Up @@ -92,9 +93,9 @@ def get_mergeable_mrs(self, merge_requests):
mergeable_mrs.append(merge_request)
return mergeable_mrs

def push_batch(self):
def push_batch(self, target_branch):
log.info('Pushing batch branch')
self._repo.push(BatchMergeJob.BATCH_BRANCH_NAME, force=True)
self._repo.push(f'{BatchMergeJob.BATCH_BRANCH_NAME}/{target_branch}', force=True)

def ensure_mr_not_changed(self, merge_request):
log.info('Ensuring MR !%s did not change', merge_request.iid)
Expand Down Expand Up @@ -181,11 +182,13 @@ def accept_mr(
return final_sha

def execute(self):
# Cleanup previous batch work
self.remove_batch_branch()
self.close_batch_mr()

target_branch = self._merge_requests[0].target_branch

# Cleanup previous batch work
self.remove_batch_branch(target_branch)
self.close_batch_mr(target_branch)

merge_requests = self.get_mrs_with_common_target_branch(target_branch)
merge_requests = self.get_mergeable_mrs(merge_requests)

Expand All @@ -201,7 +204,8 @@ def execute(self):
remote_target_branch_sha = self._repo.get_commit_hash('origin/%s' % target_branch)

self._repo.checkout_branch(target_branch, 'origin/%s' % target_branch)
self._repo.checkout_branch(BatchMergeJob.BATCH_BRANCH_NAME, 'origin/%s' % target_branch)
self._repo.checkout_branch(f'{BatchMergeJob.BATCH_BRANCH_NAME}/{target_branch}',
'origin/%s' % target_branch)

working_merge_requests = []

Expand Down Expand Up @@ -241,7 +245,7 @@ def execute(self):
raise CannotBatch('not enough ready merge requests')
if self._project.only_allow_merge_if_pipeline_succeeds:
# This switches git to <batch> branch
self.push_batch()
self.push_batch(target_branch)
batch_mr = self.create_batch_mr(
target_branch=target_branch,
)
Expand Down
13 changes: 7 additions & 6 deletions tests/test_batch_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ def get_batch_merge_job(self, api, mocklab, **batch_merge_kwargs):
def test_remove_batch_branch(self, api, mocklab):
repo = create_autospec(marge.git.Repo, spec_set=True)
batch_merge_job = self.get_batch_merge_job(api, mocklab, repo=repo)
batch_merge_job.remove_batch_branch()
batch_merge_job.remove_batch_branch('master')
repo.remove_branch.assert_called_once_with(
BatchMergeJob.BATCH_BRANCH_NAME,
f'{BatchMergeJob.BATCH_BRANCH_NAME}/master',
)

def test_close_batch_mr(self, api, mocklab):
Expand All @@ -60,9 +60,10 @@ def test_close_batch_mr(self, api, mocklab):
mr_class.search.return_value = [batch_mr]

batch_merge_job = self.get_batch_merge_job(api, mocklab)
batch_merge_job.close_batch_mr()
batch_merge_job.close_batch_mr('master')

params = {
'target_branch': 'master',
'author_id': batch_merge_job._user.id,
'labels': BatchMergeJob.BATCH_BRANCH_NAME,
'state': 'opened',
Expand All @@ -86,7 +87,7 @@ def test_create_batch_mr(self, api, mocklab):
r_batch_mr = batch_merge_job.create_batch_mr(target_branch)

params = {
'source_branch': BatchMergeJob.BATCH_BRANCH_NAME,
'source_branch': f'{BatchMergeJob.BATCH_BRANCH_NAME}/{target_branch}',
'target_branch': target_branch,
'title': 'Marge Bot Batch MR - DO NOT TOUCH',
'labels': BatchMergeJob.BATCH_BRANCH_NAME,
Expand Down Expand Up @@ -132,9 +133,9 @@ def test_ensure_mergeable_mr_ci_not_ok(self, bmj_get_mr_ci_status, api, mocklab)

def test_push_batch(self, api, mocklab):
batch_merge_job = self.get_batch_merge_job(api, mocklab)
batch_merge_job.push_batch()
batch_merge_job.push_batch('master')
batch_merge_job._repo.push.assert_called_once_with(
BatchMergeJob.BATCH_BRANCH_NAME,
f'{BatchMergeJob.BATCH_BRANCH_NAME}/master',
force=True,
)

Expand Down

0 comments on commit 0585293

Please sign in to comment.