summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOswaldo Ferreira <oswaldo@gitlab.com>2017-09-27 17:38:23 -0300
committerOswaldo Ferreira <oswaldo@gitlab.com>2017-09-27 17:56:51 -0300
commite38dc10c09ac6e6d7752d2bb9eeb0c3feab9765a (patch)
treef7dc3283615bb00dc81c5b12196d2cf3512a7fec
parent38a848be1c1ea5cba3112def8b3d64a2af056f0e (diff)
downloadgitlab-ce-e38dc10c09ac6e6d7752d2bb9eeb0c3feab9765a.tar.gz
Clean merge_jid whenever necessary on the merge process38476-improve-merge-jid-cleanup-on-merge-process
MergeRequest#merge_jid should be cleaned up whenever we hit a known error on MergeService#execute. This way we can keep track if the MR is really "ongoing" or "stuck"
-rw-r--r--app/services/merge_requests/merge_service.rb19
-rw-r--r--changelogs/unreleased/38476-improve-merge-jid-cleanup-on-merge-process.yml5
-rw-r--r--spec/services/merge_requests/merge_service_spec.rb23
3 files changed, 37 insertions, 10 deletions
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb
index 07cbd8f92a9..bf26859dd6d 100644
--- a/app/services/merge_requests/merge_service.rb
+++ b/app/services/merge_requests/merge_service.rb
@@ -14,13 +14,13 @@ module MergeRequests
@merge_request = merge_request
unless @merge_request.mergeable?
- return log_merge_error('Merge request is not mergeable', save_message_on_model: true)
+ return handle_merge_error(log_message: 'Merge request is not mergeable', save_message_on_model: true)
end
@source = find_merge_source
unless @source
- return log_merge_error('No source for merge', save_message_on_model: true)
+ return handle_merge_error(log_message: 'No source for merge', save_message_on_model: true)
end
merge_request.in_locked_state do
@@ -31,8 +31,7 @@ module MergeRequests
end
end
rescue MergeError => e
- clean_merge_jid
- log_merge_error(e.message, save_message_on_model: true)
+ handle_merge_error(log_message: e.message, save_message_on_model: true)
end
private
@@ -74,10 +73,16 @@ module MergeRequests
@merge_request.force_remove_source_branch? ? @merge_request.author : current_user
end
- def log_merge_error(message, save_message_on_model: false)
- Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{message}")
+ # Logs merge error message and cleans `MergeRequest#merge_jid`.
+ #
+ def handle_merge_error(log_message:, save_message_on_model: false)
+ Rails.logger.error("MergeService ERROR: #{merge_request_info} - #{log_message}")
- @merge_request.update(merge_error: message) if save_message_on_model
+ if save_message_on_model
+ @merge_request.update(merge_error: log_message, merge_jid: nil)
+ else
+ clean_merge_jid
+ end
end
def merge_request_info
diff --git a/changelogs/unreleased/38476-improve-merge-jid-cleanup-on-merge-process.yml b/changelogs/unreleased/38476-improve-merge-jid-cleanup-on-merge-process.yml
new file mode 100644
index 00000000000..43dec51029b
--- /dev/null
+++ b/changelogs/unreleased/38476-improve-merge-jid-cleanup-on-merge-process.yml
@@ -0,0 +1,5 @@
+---
+title: Adjust MRs being stuck on "process of being merged" for more than 2 hours
+merge_request:
+author:
+type: fixed
diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index b60136064b7..80213d093f1 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/spec/services/merge_requests/merge_service_spec.rb
@@ -13,20 +13,21 @@ describe MergeRequests::MergeService do
describe '#execute' do
context 'MergeRequest#merge_jid' do
+ let(:service) do
+ described_class.new(project, user, commit_message: 'Awesome message')
+ end
+
before do
merge_request.update_column(:merge_jid, 'hash-123')
end
it 'is cleaned when no error is raised' do
- service = described_class.new(project, user, commit_message: 'Awesome message')
-
service.execute(merge_request)
expect(merge_request.reload.merge_jid).to be_nil
end
it 'is cleaned when expected error is raised' do
- service = described_class.new(project, user, commit_message: 'Awesome message')
allow(service).to receive(:commit).and_raise(described_class::MergeError)
service.execute(merge_request)
@@ -34,6 +35,22 @@ describe MergeRequests::MergeService do
expect(merge_request.reload.merge_jid).to be_nil
end
+ it 'is cleaned when merge request is not mergeable' do
+ allow(merge_request).to receive(:mergeable?).and_return(false)
+
+ service.execute(merge_request)
+
+ expect(merge_request.reload.merge_jid).to be_nil
+ end
+
+ it 'is cleaned when no source is found' do
+ allow(merge_request).to receive(:diff_head_sha).and_return(nil)
+
+ service.execute(merge_request)
+
+ expect(merge_request.reload.merge_jid).to be_nil
+ end
+
it 'is not cleaned when unexpected error is raised' do
service = described_class.new(project, user, commit_message: 'Awesome message')
allow(service).to receive(:commit).and_raise(StandardError)