Commit d61d11fc authored by David Kim's avatar David Kim

Process LFS list request in batches to avoid request errors

parent 2a3770cd
...@@ -7,6 +7,10 @@ module Projects ...@@ -7,6 +7,10 @@ module Projects
class LfsDownloadLinkListService < BaseService class LfsDownloadLinkListService < BaseService
DOWNLOAD_ACTION = 'download' DOWNLOAD_ACTION = 'download'
# This could be different per server, but it seems like a reasonable value to start with.
# https://github.com/git-lfs/git-lfs/issues/419
REQUEST_BATCH_SIZE = 100
DownloadLinksError = Class.new(StandardError) DownloadLinksError = Class.new(StandardError)
DownloadLinkNotFound = Class.new(StandardError) DownloadLinkNotFound = Class.new(StandardError)
...@@ -25,7 +29,13 @@ module Projects ...@@ -25,7 +29,13 @@ module Projects
def execute(oids) def execute(oids)
return [] unless project&.lfs_enabled? && remote_uri && oids.present? return [] unless project&.lfs_enabled? && remote_uri && oids.present?
get_download_links(oids) download_links = []
oids.each_slice(REQUEST_BATCH_SIZE) do |batch|
download_links += get_download_links(batch)
end
download_links
end end
private private
......
...@@ -8,8 +8,8 @@ describe Projects::LfsPointers::LfsDownloadLinkListService do ...@@ -8,8 +8,8 @@ describe Projects::LfsPointers::LfsDownloadLinkListService do
let(:new_oids) { { 'oid1' => 123, 'oid2' => 125 } } let(:new_oids) { { 'oid1' => 123, 'oid2' => 125 } }
let(:remote_uri) { URI.parse(lfs_endpoint) } let(:remote_uri) { URI.parse(lfs_endpoint) }
let(:objects_response) do def objects_response(oids)
body = new_oids.map do |oid, size| body = oids.map do |oid, size|
{ {
'oid' => oid, 'oid' => oid,
'size' => size, 'size' => size,
...@@ -19,7 +19,7 @@ describe Projects::LfsPointers::LfsDownloadLinkListService do ...@@ -19,7 +19,7 @@ describe Projects::LfsPointers::LfsDownloadLinkListService do
} }
end end
Struct.new(:success?, :objects).new(true, body) Struct.new(:success?, :objects).new(true, body).to_json
end end
let(:invalid_object_response) do let(:invalid_object_response) do
...@@ -34,7 +34,7 @@ describe Projects::LfsPointers::LfsDownloadLinkListService do ...@@ -34,7 +34,7 @@ describe Projects::LfsPointers::LfsDownloadLinkListService do
before do before do
allow(project).to receive(:lfs_enabled?).and_return(true) allow(project).to receive(:lfs_enabled?).and_return(true)
response = instance_double(Gitlab::HTTP::Response) response = instance_double(Gitlab::HTTP::Response)
allow(response).to receive(:body).and_return(objects_response.to_json) allow(response).to receive(:body).and_return(objects_response(new_oids))
allow(response).to receive(:success?).and_return(true) allow(response).to receive(:success?).and_return(true)
allow(Gitlab::HTTP).to receive(:post).and_return(response) allow(Gitlab::HTTP).to receive(:post).and_return(response)
end end
...@@ -46,6 +46,35 @@ describe Projects::LfsPointers::LfsDownloadLinkListService do ...@@ -46,6 +46,35 @@ describe Projects::LfsPointers::LfsDownloadLinkListService do
end end
end end
context 'when lfs objects size is larger than the batch size' do
def stub_request(batch)
response = instance_double Gitlab::HTTP::Response, body: objects_response(batch), success?: true
expect(Gitlab::HTTP).to receive(:post).with(
remote_uri,
{
body: { operation: 'download', objects: batch.map { |k, v| { oid: k, size: v } } }.to_json,
headers: subject.send(:headers)
}
).and_return(response)
end
let(:new_oids) { { 'oid1' => 123, 'oid2' => 125, 'oid3' => 126, 'oid4' => 127, 'oid5' => 128 } }
before do
stub_const("#{described_class.name}::REQUEST_BATCH_SIZE", 2)
stub_request([['oid1', 123], ['oid2', 125]])
stub_request([['oid3', 126], ['oid4', 127]])
stub_request([['oid5', 128]])
end
it 'retreives them in batches' do
subject.execute(new_oids).each do |lfs_download_object|
expect(lfs_download_object.link).to eq "#{import_url}/gitlab-lfs/objects/#{lfs_download_object.oid}"
end
end
end
context 'credentials' do context 'credentials' do
context 'when the download link and the lfs_endpoint have the same host' do context 'when the download link and the lfs_endpoint have the same host' do
context 'when lfs_endpoint has credentials' do context 'when lfs_endpoint has credentials' do
......
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