Skip to content

Commit

Permalink
feat: enable multiple target branches for batching MRs (#267)
Browse files Browse the repository at this point in the history
In order to support batch jobs in parallel, the batch branches
need to be tied to the target branch name.

Signed-off-by: Sascha Binckly <[email protected]>
  • Loading branch information
sg70 committed Mar 31, 2022
1 parent 70ab3c8 commit d9354ac
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 30 deletions.
46 changes: 24 additions & 22 deletions marge/batch_job.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,24 @@ class CannotBatch(Exception):


class BatchMergeJob(MergeJob):
BATCH_BRANCH_NAME = 'marge_bot_batch_merge_job'
BATCH_BRANCH_NAME = 'marge_bot_batch_merge_job_'

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, batch_branch_name):
log.info('Removing local batch branch')
try:
self._repo.remove_branch(BatchMergeJob.BATCH_BRANCH_NAME)
self._repo.remove_branch(batch_branch_name)
except git.GitError:
pass

def close_batch_mr(self):
def close_batch_mr(self, batch_branch_name):
log.info('Closing batch MRs')
params = {
'author_id': self._user.id,
'labels': BatchMergeJob.BATCH_BRANCH_NAME,
'labels': batch_branch_name,
'state': 'opened',
'order_by': 'created_at',
'sort': 'desc',
Expand All @@ -46,14 +46,14 @@ def close_batch_mr(self):
log.info('Closing batch MR !%s', batch_mr.iid)
batch_mr.close()

def create_batch_mr(self, target_branch):
self.push_batch()
def create_batch_mr(self, target_branch, batch_branch_name):
self.push_batch(batch_branch_name)
log.info('Creating batch MR')
params = {
'source_branch': BatchMergeJob.BATCH_BRANCH_NAME,
'source_branch': batch_branch_name,
'target_branch': target_branch,
'title': 'Marge Bot Batch MR - DO NOT TOUCH',
'labels': BatchMergeJob.BATCH_BRANCH_NAME,
'labels': batch_branch_name,
}
batch_mr = MergeRequest.create(
api=self._api,
Expand Down Expand Up @@ -94,9 +94,9 @@ def get_mergeable_mrs(self, merge_requests):
mergeable_mrs.append(merge_request)
return mergeable_mrs

def push_batch(self):
def push_batch(self, batch_branch_name):
log.info('Pushing batch branch')
self._repo.push(BatchMergeJob.BATCH_BRANCH_NAME, force=True)
self._repo.push(batch_branch_name, 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 @@ -198,11 +198,15 @@ def accept_mr(
return final_sha

def execute(self):
# name of batch branch based on the name of target branch to allow parallel merges #267
target_branch = self._merge_requests[0].target_branch
batch_branch_name = f'{BatchMergeJob.BATCH_BRANCH_NAME}{target_branch}'
log.debug("batch: execute: batch_branch_name: %s", batch_branch_name)

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

target_branch = self._merge_requests[0].target_branch
merge_requests = self.get_mrs_with_common_target_branch(target_branch)
merge_requests = self.get_mergeable_mrs(merge_requests)

Expand All @@ -218,11 +222,9 @@ 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(batch_branch_name, 'origin/%s' % target_branch)

batch_mr = self.create_batch_mr(
target_branch=target_branch,
)
batch_mr = self.create_batch_mr(target_branch, batch_branch_name)
batch_mr_sha = batch_mr.sha

working_merge_requests = []
Expand All @@ -243,7 +245,7 @@ def execute(self):
)
# Update <batch> branch with MR changes
batch_mr_sha = self._repo.merge(
BatchMergeJob.BATCH_BRANCH_NAME,
batch_branch_name,
merge_request.source_branch,
'-m',
'Batch merge !%s into %s (!%s)' % (
Expand All @@ -257,13 +259,13 @@ def execute(self):
# Update <source_branch> on latest <batch> branch so it contains previous MRs
self.fuse(
merge_request.source_branch,
BatchMergeJob.BATCH_BRANCH_NAME,
batch_branch_name,
source_repo_url=source_repo_url,
local=True,
)
# Update <batch> branch with MR changes
batch_mr_sha = self._repo.fast_forward(
BatchMergeJob.BATCH_BRANCH_NAME,
batch_branch_name,
merge_request.source_branch,
local=True,
)
Expand All @@ -286,7 +288,7 @@ def execute(self):
raise CannotBatch('not enough ready merge requests')

# This switches git to <batch> branch
self.push_batch()
self.push_batch(batch_branch_name)
for merge_request in working_merge_requests:
merge_request.comment('I will attempt to batch this MR (!{})...'.format(batch_mr.iid))

Expand Down
19 changes: 11 additions & 8 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(f'{BatchMergeJob.BATCH_BRANCH_NAME}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,7 +60,7 @@ 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(BatchMergeJob.BATCH_BRANCH_NAME)

params = {
'author_id': batch_merge_job._user.id,
Expand All @@ -83,13 +83,16 @@ def test_create_batch_mr(self, api, mocklab):

batch_merge_job = self.get_batch_merge_job(api, mocklab)
target_branch = 'master'
r_batch_mr = batch_merge_job.create_batch_mr(target_branch)
r_batch_mr = batch_merge_job.create_batch_mr(
target_branch,
f'{BatchMergeJob.BATCH_BRANCH_NAME}{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,
'labels': f'{BatchMergeJob.BATCH_BRANCH_NAME}{target_branch}',
}
mr_class.create.assert_called_once_with(
api=ANY,
Expand Down Expand Up @@ -132,9 +135,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(f'{BatchMergeJob.BATCH_BRANCH_NAME}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 d9354ac

Please sign in to comment.