Commit 130e060e authored by Tiago Botelho's avatar Tiago Botelho

Supresses error being raised due to async remove remote being run outside a transaction

parent 94d7e98e
......@@ -18,13 +18,26 @@ module Sidekiq
%i(perform_async perform_at perform_in).each do |name|
define_method(name) do |*args|
if !Sidekiq::Worker.skip_transaction_check && AfterCommitQueue.inside_transaction?
raise Sidekiq::Worker::EnqueueFromTransactionError, <<~MSG
begin
raise Sidekiq::Worker::EnqueueFromTransactionError, <<~MSG
`#{self}.#{name}` cannot be called inside a transaction as this can lead to
race conditions when the worker runs before the transaction is committed and
tries to access a model that has not been saved yet.
Use an `after_commit` hook, or include `AfterCommitQueue` and use a `run_after_commit` block instead.
MSG
MSG
rescue Sidekiq::Worker::EnqueueFromTransactionError => e
if Rails.env.production?
Rails.logger.error(e.message)
if Gitlab::Sentry.enabled?
Gitlab::Sentry.context
Raven.capture_exception(e)
end
else
raise
end
end
end
super(*args)
......
......@@ -337,7 +337,9 @@ module EE
end
def remove_mirror_repository_reference
repository.async_remove_remote(::Repository::MIRROR_REMOTE)
run_after_commit do
repository.async_remove_remote(::Repository::MIRROR_REMOTE)
end
end
def username_only_import_url
......
......@@ -24,7 +24,8 @@ class RemoteMirror < ActiveRecord::Base
after_save :set_override_remote_mirror_available, unless: -> { Gitlab::CurrentSettings.current_application_settings.mirror_available }
after_save :refresh_remote, if: :mirror_url_changed?
after_update :reset_fields, if: :mirror_url_changed?
after_destroy :remove_remote
after_commit :remove_remote, on: :destroy
scope :enabled, -> { where(enabled: true) }
scope :started, -> { with_update_status(:started) }
......
---
title: Supresses error being raised due to async remote removal being run outside
a transaction
merge_request: 4747
author:
type: fixed
......@@ -328,6 +328,16 @@ describe Project do
end
end
describe 'updating import_url' do
it 'removes previous remote' do
project = create(:project, :repository, :mirror)
expect(RepositoryRemoveRemoteWorker).to receive(:perform_async).with(project.id, ::Repository::MIRROR_REMOTE).and_call_original
project.update_attributes(import_url: "http://test.com")
end
end
describe '#mirror_waiting_duration' do
it 'returns in seconds the time spent in the queue' do
project = create(:project, :mirror, :import_scheduled)
......
......@@ -70,6 +70,14 @@ describe RemoteMirror do
config = repo.raw_repository.rugged.config
expect(config["remote.#{mirror.remote_name}.url"]).to eq('http://foo:baz@test.com')
end
it 'removes previous remote' do
mirror = create_mirror(url: 'http://foo:bar@test.com')
expect(RepositoryRemoveRemoteWorker).to receive(:perform_async).with(mirror.project.id, mirror.remote_name).and_call_original
mirror.update_attributes(url: 'http://test.com')
end
end
end
......@@ -120,6 +128,16 @@ describe RemoteMirror do
end
end
context 'when remote mirror gets destroyed' do
it 'removes remote' do
mirror = create_mirror(url: 'http://foo:bar@test.com')
expect(RepositoryRemoveRemoteWorker).to receive(:perform_async).with(mirror.project.id, mirror.remote_name).and_call_original
mirror.destroy!
end
end
context 'stuck mirrors' do
it 'includes mirrors stuck in started with no last_update_at set' do
mirror = create_mirror(url: 'http://cantbeblank',
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment