Commit 25db1037 authored by Aleksei Lipniagov's avatar Aleksei Lipniagov Committed by Kamil Trzciński

Project Import: Fail gracefully & record failures

 - Add feature flag import_graceful_failures
 - Add feature flag to group instead of project
 - Add import_failures table
 - Gracefully fail, and log error
 - Send error to Sentry
parent 97172bbe
# frozen_string_literal: true
class ImportFailure < ApplicationRecord
belongs_to :project
validates :project, presence: true
end
......@@ -295,6 +295,8 @@ class Project < ApplicationRecord
has_one :pages_metadatum, class_name: 'ProjectPagesMetadatum', inverse_of: :project
has_many :import_failures, inverse_of: :project
accepts_nested_attributes_for :variables, allow_destroy: true
accepts_nested_attributes_for :project_feature, update_only: true
accepts_nested_attributes_for :import_data
......
---
title: Collect project import failures instead of failing fast
merge_request: 20727
author:
type: other
# frozen_string_literal: true
class CreateImportFailures < ActiveRecord::Migration[5.2]
DOWNTIME = false
def change
create_table :import_failures do |t|
t.integer :relation_index
t.references :project, null: false, index: true
t.datetime_with_timezone :created_at, null: false
t.string :relation_key, limit: 64
t.string :exception_class, limit: 128
t.string :correlation_id_value, limit: 128, index: true
t.string :exception_message, limit: 255
end
end
end
......@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2019_11_24_150431) do
ActiveRecord::Schema.define(version: 2019_11_25_140458) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_trgm"
......@@ -1948,6 +1948,18 @@ ActiveRecord::Schema.define(version: 2019_11_24_150431) do
t.index ["updated_at"], name: "index_import_export_uploads_on_updated_at"
end
create_table "import_failures", force: :cascade do |t|
t.integer "relation_index"
t.bigint "project_id", null: false
t.datetime_with_timezone "created_at", null: false
t.string "relation_key", limit: 64
t.string "exception_class", limit: 128
t.string "correlation_id_value", limit: 128
t.string "exception_message", limit: 255
t.index ["correlation_id_value"], name: "index_import_failures_on_correlation_id_value"
t.index ["project_id"], name: "index_import_failures_on_project_id"
end
create_table "index_statuses", id: :serial, force: :cascade do |t|
t.integer "project_id", null: false
t.datetime "indexed_at"
......
......@@ -20,7 +20,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
restored_project_json
end
it_behaves_like 'restores project correctly', issues: 2
it_behaves_like 'restores project successfully', issues: 2
it 'restores project associations correctly' do
expect(project.designs.size).to eq(7)
......
......@@ -85,13 +85,16 @@ module Gitlab
# we do not care if we process array or hash
data_hashes = [data_hashes] unless data_hashes.is_a?(Array)
relation_index = 0
# consume and remove objects from memory
while data_hash = data_hashes.shift
process_project_relation_item!(relation_key, relation_definition, data_hash)
process_project_relation_item!(relation_key, relation_definition, relation_index, data_hash)
relation_index += 1
end
end
def process_project_relation_item!(relation_key, relation_definition, data_hash)
def process_project_relation_item!(relation_key, relation_definition, relation_index, data_hash)
relation_object = build_relation(relation_key, relation_definition, data_hash)
return unless relation_object
return if group_model?(relation_object)
......@@ -100,6 +103,25 @@ module Gitlab
relation_object.save!
save_id_mapping(relation_key, data_hash, relation_object)
rescue => e
# re-raise if not enabled
raise e unless Feature.enabled?(:import_graceful_failures, @project.group)
log_import_failure(relation_key, relation_index, e)
end
def log_import_failure(relation_key, relation_index, exception)
Gitlab::Sentry.track_acceptable_exception(exception,
extra: { project_id: @project.id, relation_key: relation_key, relation_index: relation_index })
ImportFailure.create(
project: @project,
relation_key: relation_key,
relation_index: relation_index,
exception_class: exception.class.to_s,
exception_message: exception.message.truncate(255),
correlation_id_value: Labkit::Correlation::CorrelationId.current_id
)
end
# Older, serialized CI pipeline exports may only have a
......
......@@ -43,6 +43,7 @@ describe 'Database schema' do
geo_nodes: %w[oauth_application_id],
geo_repository_deleted_events: %w[project_id],
geo_upload_deleted_events: %w[upload_id model_id],
import_failures: %w[project_id],
identities: %w[user_id],
issues: %w[last_edited_by_id state_id],
jira_tracker_data: %w[jira_issue_transition_id],
......
{
"description": "Nisi et repellendus ut enim quo accusamus vel magnam.",
"import_type": "gitlab_project",
"creator_id": 123,
"visibility_level": 10,
"archived": false,
"milestones": [
{
"id": 1,
"title": null,
"project_id": 8,
"description": 123,
"due_date": null,
"created_at": "NOT A DATE",
"updated_at": "NOT A DATE",
"state": "active",
"iid": 1,
"group_id": null
},
{
"id": 42,
"title": "A valid milestone",
"project_id": 8,
"description": "Project-level milestone",
"due_date": null,
"created_at": "2016-06-14T15:02:04.415Z",
"updated_at": "2016-06-14T15:02:04.415Z",
"state": "active",
"iid": 1,
"group_id": null
}
],
"labels": [],
"issues": [],
"services": [],
"snippets": [],
"hooks": []
}
......@@ -434,6 +434,7 @@ project:
- upstream_project_subscriptions
- downstream_project_subscriptions
- service_desk_setting
- import_failures
award_emoji:
- awardable
- user
......
......@@ -362,7 +362,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
expect(restored_project_json).to eq(true)
end
it_behaves_like 'restores project correctly',
it_behaves_like 'restores project successfully',
issues: 1,
labels: 2,
label_with_priorities: 'A project label',
......@@ -375,7 +375,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
create(:ci_build, token: 'abcd')
end
it_behaves_like 'restores project correctly',
it_behaves_like 'restores project successfully',
issues: 1,
labels: 2,
label_with_priorities: 'A project label',
......@@ -452,7 +452,7 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
expect(restored_project_json).to eq(true)
end
it_behaves_like 'restores project correctly',
it_behaves_like 'restores project successfully',
issues: 2,
labels: 2,
label_with_priorities: 'A project label',
......@@ -633,4 +633,46 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
end
end
end
context 'JSON with invalid records' do
let(:user) { create(:user) }
let!(:project) { create(:project, :builds_disabled, :issues_disabled, name: 'project', path: 'project') }
let(:project_tree_restorer) { described_class.new(user: user, shared: shared, project: project) }
let(:restored_project_json) { project_tree_restorer.restore }
context 'when some failures occur' do
context 'because a relation fails to be processed' do
let(:correlation_id) { 'my-correlation-id' }
before do
setup_import_export_config('with_invalid_records')
Labkit::Correlation::CorrelationId.use_id(correlation_id) do
expect(restored_project_json).to eq(true)
end
end
it_behaves_like 'restores project successfully',
issues: 0,
labels: 0,
label_with_priorities: nil,
milestones: 1,
first_issue_labels: 0,
services: 0,
import_failures: 1
it 'records the failures in the database' do
import_failure = ImportFailure.last
expect(import_failure.project_id).to eq(project.id)
expect(import_failure.relation_key).to eq('milestones')
expect(import_failure.relation_index).to be_present
expect(import_failure.exception_class).to eq('ActiveRecord::RecordInvalid')
expect(import_failure.exception_message).to be_present
expect(import_failure.correlation_id_value).to eq('my-correlation-id')
expect(import_failure.created_at).to be_present
end
end
end
end
end
......@@ -2,7 +2,7 @@
# Shared examples for ProjectTreeRestorer (shared to allow the testing
# of EE-specific features)
RSpec.shared_examples 'restores project correctly' do |**results|
RSpec.shared_examples 'restores project successfully' do |**results|
it 'restores the project' do
expect(shared.errors).to be_empty
expect(restored_project_json).to be_truthy
......@@ -34,4 +34,8 @@ RSpec.shared_examples 'restores project correctly' do |**results|
expect(project.import_type).to be_nil
expect(project.creator_id).not_to eq 123
end
it 'records exact number of import failures' do
expect(project.import_failures.size).to eq(results.fetch(:import_failures, 0))
end
end
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