diff --git a/marge/batch_job.py b/marge/batch_job.py index faf6b55f..280c7454 100644 --- a/marge/batch_job.py +++ b/marge/batch_job.py @@ -1,6 +1,6 @@ # pylint: disable=too-many-branches,too-many-statements,arguments-differ import logging as log -from time import sleep +import time from . import git from . import gitlab @@ -110,17 +110,19 @@ def ensure_mr_not_changed(self, merge_request): if getattr(changed_mr, attr) != getattr(merge_request, attr): raise CannotMerge(error_message.format(attr.replace('_', ' '))) - def merge_batch(self, target_branch, source_branch, no_ff=False): + def merge_batch(self, target_branch, source_branch, no_ff=False, source_repo_url=None, local=False): if no_ff: return self._repo.merge( target_branch, source_branch, '--no-ff', + source_repo_url, ) return self._repo.fast_forward( target_branch, source_branch, + local=local ) def update_merge_request( @@ -166,22 +168,29 @@ def accept_mr( if new_target_sha != expected_remote_target_branch_sha: raise CannotBatch('Someone was naughty and by-passed marge') + log.debug("batch: accept_mr: self.update_merge_request") # Rebase and apply the trailers self.update_merge_request( merge_request, source_repo_url=source_repo_url, ) - # This switches git to + log.debug("batch: accept_mr: self.merge_batch") final_sha = self.merge_batch( merge_request.target_branch, merge_request.source_branch, self._options.use_no_ff_batches, + source_repo_url, + local=True # since we're merging multiple MRs from forks, local needs to be used ) + log.debug("batch: accept_mr: self._repo.push") # Don't force push in case the remote has changed. self._repo.push(merge_request.target_branch, force=False) - sleep(2) + # delay in case Gitlab is slow to respond + waiting_time_in_secs = 10 + log.debug('Waiting for %s secs for Gitlab to start CI after push', waiting_time_in_secs) + time.sleep(waiting_time_in_secs) # At this point Gitlab should have recognised the MR as being accepted. log.info('Successfully merged MR !%s', merge_request.iid) @@ -192,6 +201,9 @@ def accept_mr( branch=merge_request.source_branch, status='running', ) + + log.debug("batch: accept_mr: canceling pipelines in Gitlab") + # NOTE: this might not abort the pipeline running in external CI, it merely disconnects the pipeline for pipeline in pipelines: pipeline.cancel() @@ -225,6 +237,11 @@ def execute(self): ) batch_mr_sha = batch_mr.sha + # delay in case Gitlab is slow to respond + waiting_time_in_secs = 20 + log.debug('Waiting for %s secs for MR !%s to get status update', waiting_time_in_secs, batch_mr.iid) + time.sleep(waiting_time_in_secs) + working_merge_requests = [] for merge_request in merge_requests: @@ -290,6 +307,11 @@ def execute(self): for merge_request in working_merge_requests: merge_request.comment('I will attempt to batch this MR (!{})...'.format(batch_mr.iid)) + # delay in case Gitlab is slow to respond + waiting_time_in_secs = 10 + log.debug('Waiting for %s secs for Gitlab to start CI after push', waiting_time_in_secs) + time.sleep(waiting_time_in_secs) + # wait for the CI of the batch MR if self._project.only_allow_merge_if_pipeline_succeeds: try: diff --git a/marge/git.py b/marge/git.py index a46b023e..5199ad13 100644 --- a/marge/git.py +++ b/marge/git.py @@ -104,6 +104,9 @@ def rebase(self, branch, new_base, source_repo_url=None, local=False): def _fuse_branch(self, strategy, branch, target_branch, *fuse_args, source_repo_url=None, local=False): assert source_repo_url or branch != target_branch, branch + log.debug("git: _fuse_branch: %s %s %s %s", branch, target_branch, source_repo_url, local) + + log.warning("git: _fuse_branch: local: %s", local) if not local: self.fetch('origin') target = 'origin/' + target_branch diff --git a/marge/gitlab.py b/marge/gitlab.py index e025f704..5e8ce298 100644 --- a/marge/gitlab.py +++ b/marge/gitlab.py @@ -16,7 +16,7 @@ def call(self, command, sudo=None): headers = {'PRIVATE-TOKEN': self._auth_token} if sudo: headers['SUDO'] = '%d' % sudo - log.debug('REQUEST: %s %s %r %r', method.__name__.upper(), url, headers, command.call_args) + log.debug('REQUEST: %s %s %r', method.__name__.upper(), url, command.call_args) # Timeout to prevent indefinitely hanging requests. 60s is very conservative, # but should be short enough to not cause any practical annoyances. We just # crash rather than retry since marge-bot should be run in a restart loop anyway. diff --git a/tests/test_batch_job.py b/tests/test_batch_job.py index 4ac2c5bc..c9cbcf35 100644 --- a/tests/test_batch_job.py +++ b/tests/test_batch_job.py @@ -142,10 +142,22 @@ def test_merge_batch(self, api, mocklab): batch_merge_job = self.get_batch_merge_job(api, mocklab) target_branch = 'master' source_branch = mocklab.merge_request_info['source_branch'] - batch_merge_job.merge_batch(target_branch, source_branch, no_ff=False) + batch_merge_job.merge_batch(target_branch, source_branch, no_ff=False, local=False) batch_merge_job._repo.fast_forward.assert_called_once_with( target_branch, source_branch, + local=False + ) + + def test_merge_batch_forks(self, api, mocklab): + batch_merge_job = self.get_batch_merge_job(api, mocklab) + target_branch = 'master' + source_branch = mocklab.merge_request_info['source_branch'] + batch_merge_job.merge_batch(target_branch, source_branch, no_ff=False, local=True) + batch_merge_job._repo.fast_forward.assert_called_once_with( + target_branch, + source_branch, + local=True ) def test_merge_batch_with_no_ff_enabled(self, api, mocklab): @@ -156,7 +168,8 @@ def test_merge_batch_with_no_ff_enabled(self, api, mocklab): batch_merge_job._repo.merge.assert_called_once_with( target_branch, source_branch, - '--no-ff' + '--no-ff', + None, ) batch_merge_job._repo.fast_forward.assert_not_called()