feat: enable multiple target branches for batching MRs (#267)

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]>
smarkets · Mar 31, 2022 · a0e44b7 · a0e44b7
1 parent 70ab3c8
commit a0e44b7
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 28 deletions.
diff --git a/marge/batch_job.py b/marge/batch_job.py
@@ -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',
@@ -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,
@@ -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)
@@ -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)
 
@@ -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 = []
@@ -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)' % (
@@ -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,
                     )
@@ -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))
 

diff --git a/tests/test_batch_job.py b/tests/test_batch_job.py
@@ -49,9 +49,11 @@ 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(BatchMergeJob.BATCH_BRANCH_NAME)
+        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):
@@ -60,7 +62,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,
@@ -83,13 +85,13 @@ 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,
@@ -132,9 +134,11 @@ 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(BatchMergeJob.BATCH_BRANCH_NAME)
+        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,
         )