Commit ff9cdc8f authored by Steve Abrams's avatar Steve Abrams Committed by Jan Provaznik

Set error status on failed package extraction

parent 093f5a50
......@@ -17,6 +17,7 @@ module Packages
name: name,
package_files: @package.package_files.map { |pf| build_package_file_view(pf) },
package_type: @package.package_type,
status: @package.status,
project_id: @package.project_id,
tags: @package.tags.as_json,
updated_at: @package.updated_at,
......
......@@ -16,6 +16,7 @@ module Packages
end
def execute
raise ExtractionError, 'Gem was not processed - package_file is not set' unless package_file
return success if process_gem
error('Gem was not processed')
......@@ -26,8 +27,6 @@ module Packages
attr_reader :package_file
def process_gem
return false unless package_file
try_obtain_lease do
package.transaction do
rename_package_and_set_version
......
......@@ -1354,7 +1354,7 @@
:urgency: :low
:resource_boundary: :unknown
:weight: 1
:idempotent: true
:idempotent:
:tags:
- :exclude_from_kubernetes
- :name: pipeline_background:archive_trace
......
......@@ -20,7 +20,7 @@ module Packages
rescue ::Packages::Nuget::MetadataExtractionService::ExtractionError,
::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError => e
Gitlab::ErrorTracking.log_exception(e, project_id: package_file.project_id)
package_file.package.destroy!
package_file.package.update_column(:status, :error)
end
end
end
......
......@@ -12,8 +12,6 @@ module Packages
tags :exclude_from_kubernetes
deduplicate :until_executing
idempotent!
def perform(package_file_id)
package_file = ::Packages::PackageFile.find_by_id(package_file_id)
......@@ -23,7 +21,7 @@ module Packages
rescue ::Packages::Rubygems::ProcessGemService::ExtractionError => e
Gitlab::ErrorTracking.log_exception(e, project_id: package_file.project_id)
package_file.package.destroy!
package_file.package.update_column(:status, :error)
end
end
end
......
---
title: Update RubyGems and NuGet packages to error status upon metadata extraction
failure
merge_request: 60172
author:
type: changed
......@@ -22,6 +22,7 @@ module API
expose :version
expose :package_type
expose :status
expose :_links do
expose :web_path do |package|
......
......@@ -4,6 +4,7 @@
"name",
"version",
"package_type",
"status",
"_links",
"versions"
],
......@@ -20,6 +21,9 @@
"package_type": {
"type": "string"
},
"status": {
"type": "string"
},
"_links": {
"type": "object",
"required": [
......
......@@ -50,6 +50,7 @@ RSpec.describe ::Packages::Detail::PackagePresenter do
name: package.name,
package_files: expected_package_files,
package_type: package.package_type,
status: package.status,
project_id: package.project_id,
tags: package.tags.as_json,
updated_at: package.updated_at,
......
......@@ -16,12 +16,11 @@ RSpec.describe Packages::Rubygems::ProcessGemService do
describe '#execute' do
subject { service.execute }
context 'no gem file', :aggregate_failures do
context 'no gem file' do
let(:package_file) { nil }
it 'returns an error' do
expect(subject.error?).to be(true)
expect(subject.message).to eq('Gem was not processed')
expect { subject }.to raise_error(::Packages::Rubygems::ProcessGemService::ExtractionError, 'Gem was not processed - package_file is not set')
end
end
......
......@@ -14,14 +14,15 @@ RSpec.describe Packages::Nuget::ExtractionWorker, type: :worker do
subject { described_class.new.perform(package_file_id) }
shared_examples 'handling the metadata error' do |exception_class: ::Packages::Nuget::UpdatePackageFromMetadataService::InvalidMetadataError|
it 'removes the package and the package file' do
it 'updates package status to error', :aggregate_failures do
expect(Gitlab::ErrorTracking).to receive(:log_exception).with(
instance_of(exception_class),
project_id: package.project_id
)
expect { subject }
.to change { Packages::Package.count }.by(-1)
.and change { Packages::PackageFile.count }.by(-1)
subject
expect(package.reload).to be_error
end
end
......
......@@ -4,7 +4,7 @@ require 'spec_helper'
RSpec.describe Packages::Rubygems::ExtractionWorker, type: :worker do
describe '#perform' do
let_it_be(:package) { create(:rubygems_package) }
let_it_be(:package) { create(:rubygems_package, :processing) }
let(:package_file) { package.package_files.first }
let(:package_file_id) { package_file.id }
......@@ -14,15 +14,13 @@ RSpec.describe Packages::Rubygems::ExtractionWorker, type: :worker do
subject { described_class.new.perform(*job_args) }
include_examples 'an idempotent worker' do
it 'processes the gem', :aggregate_failures do
expect { subject }
.to change { Packages::Package.count }.by(0)
.and change { Packages::PackageFile.count }.by(2)
it 'processes the gem', :aggregate_failures do
expect { subject }
.to change { Packages::Package.count }.by(0)
.and change { Packages::PackageFile.count }.by(1)
expect(Packages::Package.last.id).to be(package.id)
expect(package.name).not_to be(package_name)
end
expect(Packages::Package.last.id).to be(package.id)
expect(package.name).not_to be(package_name)
end
it 'handles a processing failure', :aggregate_failures do
......@@ -34,9 +32,9 @@ RSpec.describe Packages::Rubygems::ExtractionWorker, type: :worker do
project_id: package.project_id
)
expect { subject }
.to change { Packages::Package.count }.by(-1)
.and change { Packages::PackageFile.count }.by(-2)
subject
expect(package.reload).to be_error
end
context 'returns when there is no package file' 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