Commit 64fd3fd6 authored by Kassio Borges's avatar Kassio Borges

Avoid blocking Github imports with known LFS exceptions

https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48512 recently
removed the `rescue` that ensured Github import to finish even with Lfs
exceptions.

This commit adds that rescue back.
parent 7090019c
...@@ -28,6 +28,8 @@ module Gitlab ...@@ -28,6 +28,8 @@ module Gitlab
lfs_objects.each do |object| lfs_objects.each do |object|
yield object yield object
end end
rescue StandardError => e
error(project.id, e)
end end
end end
end end
......
...@@ -50,6 +50,8 @@ module Gitlab ...@@ -50,6 +50,8 @@ module Gitlab
retval retval
rescue => e rescue => e
error(project.id, e) error(project.id, e)
raise e
end end
# Imports all the objects in sequence in the current thread. # Imports all the objects in sequence in the current thread.
...@@ -178,7 +180,7 @@ module Gitlab ...@@ -178,7 +180,7 @@ module Gitlab
) )
) )
Gitlab::ErrorTracking.track_and_raise_exception( Gitlab::ErrorTracking.track_exception(
exception, exception,
log_attributes(project_id) log_attributes(project_id)
) )
......
...@@ -49,6 +49,57 @@ RSpec.describe Gitlab::GithubImport::Importer::LfsObjectsImporter do ...@@ -49,6 +49,57 @@ RSpec.describe Gitlab::GithubImport::Importer::LfsObjectsImporter do
importer.execute importer.execute
end end
end end
context 'when LFS list download fails' do
it 'rescues and logs the known exceptions' do
exception = StandardError.new('Invalid Project URL')
importer = described_class.new(project, client, parallel: false)
expect_next_instance_of(Projects::LfsPointers::LfsObjectDownloadListService) do |service|
expect(service)
.to receive(:execute)
.and_raise(exception)
end
expect_next_instance_of(Gitlab::Import::Logger) do |logger|
expect(logger)
.to receive(:error)
.with(
message: 'importer failed',
import_source: :github,
project_id: project.id,
parallel: false,
importer: 'Gitlab::GithubImport::Importer::LfsObjectImporter',
'error.message': 'Invalid Project URL'
)
end
expect(Gitlab::ErrorTracking)
.to receive(:track_exception)
.with(
exception,
import_source: :github,
parallel: false,
project_id: project.id,
importer: 'Gitlab::GithubImport::Importer::LfsObjectImporter'
).and_call_original
importer.execute
end
it 'raises and logs the unknown exceptions' do
exception = Exception.new('Really bad news')
importer = described_class.new(project, client, parallel: false)
expect_next_instance_of(Projects::LfsPointers::LfsObjectDownloadListService) do |service|
expect(service)
.to receive(:execute)
.and_raise(exception)
end
expect { importer.execute }.to raise_error(exception)
end
end
end end
describe '#sequential_import' do describe '#sequential_import' do
...@@ -61,13 +112,11 @@ RSpec.describe Gitlab::GithubImport::Importer::LfsObjectsImporter do ...@@ -61,13 +112,11 @@ RSpec.describe Gitlab::GithubImport::Importer::LfsObjectsImporter do
end end
expect(Gitlab::GithubImport::Importer::LfsObjectImporter) expect(Gitlab::GithubImport::Importer::LfsObjectImporter)
.to receive(:new) .to receive(:new).with(
.with(
an_instance_of(Gitlab::GithubImport::Representation::LfsObject), an_instance_of(Gitlab::GithubImport::Representation::LfsObject),
project, project,
client client
) ).and_return(lfs_object_importer)
.and_return(lfs_object_importer)
expect(lfs_object_importer).to receive(:execute) expect(lfs_object_importer).to receive(:execute)
......
...@@ -131,7 +131,7 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do ...@@ -131,7 +131,7 @@ RSpec.describe Gitlab::GithubImport::ParallelScheduling do
end end
expect(Gitlab::ErrorTracking) expect(Gitlab::ErrorTracking)
.to receive(:track_and_raise_exception) .to receive(:track_exception)
.with( .with(
exception, exception,
import_source: :github, import_source: :github,
......
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