diff options
-rw-r--r-- | app/models/project.rb | 3 | ||||
-rw-r--r-- | app/models/project_services/emails_on_push_service.rb | 2 | ||||
-rw-r--r-- | app/models/project_services/irker_service.rb | 2 | ||||
-rw-r--r-- | app/models/project_services/pipelines_email_service.rb | 2 | ||||
-rw-r--r-- | app/models/service.rb | 6 | ||||
-rw-r--r-- | app/workers/repository_import_worker.rb | 10 | ||||
-rw-r--r-- | changelogs/unreleased/42327-import-from-gitlab-com-fails-destination-already-exists-and-is-not-an-empty-directory-error.yml | 6 | ||||
-rw-r--r-- | lib/gitlab/import_export/shared.rb | 7 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/project.json | 2 | ||||
-rw-r--r-- | spec/lib/gitlab/import_export/project_tree_saver_spec.rb | 6 | ||||
-rw-r--r-- | spec/workers/repository_import_worker_spec.rb | 15 |
11 files changed, 53 insertions, 8 deletions
diff --git a/app/models/project.rb b/app/models/project.rb index e19873f64ce..d0d0fd6e093 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -568,6 +568,9 @@ class Project < ActiveRecord::Base RepositoryForkWorker.perform_async(id, forked_from_project.repository_storage_path, forked_from_project.disk_path) + elsif gitlab_project_import? + # Do not retry on Import/Export until https://gitlab.com/gitlab-org/gitlab-ce/issues/26189 is solved. + RepositoryImportWorker.set(retry: false).perform_async(self.id) else RepositoryImportWorker.perform_async(self.id) end diff --git a/app/models/project_services/emails_on_push_service.rb b/app/models/project_services/emails_on_push_service.rb index 1a236e232f9..b604d860a87 100644 --- a/app/models/project_services/emails_on_push_service.rb +++ b/app/models/project_services/emails_on_push_service.rb @@ -2,7 +2,7 @@ class EmailsOnPushService < Service boolean_accessor :send_from_committer_email boolean_accessor :disable_diffs prop_accessor :recipients - validates :recipients, presence: true, if: :activated? + validates :recipients, presence: true, if: :valid_recipients? def title 'Emails on push' diff --git a/app/models/project_services/irker_service.rb b/app/models/project_services/irker_service.rb index 19357f90810..27bdf708c80 100644 --- a/app/models/project_services/irker_service.rb +++ b/app/models/project_services/irker_service.rb @@ -4,7 +4,7 @@ class IrkerService < Service prop_accessor :server_host, :server_port, :default_irc_uri prop_accessor :recipients, :channels boolean_accessor :colorize_messages - validates :recipients, presence: true, if: :activated? + validates :recipients, presence: true, if: :valid_recipients? before_validation :get_channels diff --git a/app/models/project_services/pipelines_email_service.rb b/app/models/project_services/pipelines_email_service.rb index 6a3118a11b8..9c7b58dead5 100644 --- a/app/models/project_services/pipelines_email_service.rb +++ b/app/models/project_services/pipelines_email_service.rb @@ -1,7 +1,7 @@ class PipelinesEmailService < Service prop_accessor :recipients boolean_accessor :notify_only_broken_pipelines - validates :recipients, presence: true, if: :activated? + validates :recipients, presence: true, if: :valid_recipients? def initialize_properties self.properties ||= { notify_only_broken_pipelines: true } diff --git a/app/models/service.rb b/app/models/service.rb index 96a064697f0..369cae2e85f 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -2,6 +2,8 @@ # and implement a set of methods class Service < ActiveRecord::Base include Sortable + include Importable + serialize :properties, JSON # rubocop:disable Cop/ActiveRecordSerialize default_value_for :active, false @@ -295,4 +297,8 @@ class Service < ActiveRecord::Base project.cache_has_external_wiki end end + + def valid_recipients? + activated? && !importing? + end end diff --git a/app/workers/repository_import_worker.rb b/app/workers/repository_import_worker.rb index 31e2798c36b..d79b5ee5346 100644 --- a/app/workers/repository_import_worker.rb +++ b/app/workers/repository_import_worker.rb @@ -20,7 +20,11 @@ class RepositoryImportWorker # to those importers to mark the import process as complete. return if service.async? - raise result[:message] if result[:status] == :error + if result[:status] == :error + fail_import(project, result[:message]) if project.gitlab_project_import? + + raise result[:message] + end project.after_import end @@ -33,4 +37,8 @@ class RepositoryImportWorker Rails.logger.info("Project #{project.full_path} was in inconsistent state (#{project.import_status}) while importing.") false end + + def fail_import(project, message) + project.mark_import_as_failed(message) + end end diff --git a/changelogs/unreleased/42327-import-from-gitlab-com-fails-destination-already-exists-and-is-not-an-empty-directory-error.yml b/changelogs/unreleased/42327-import-from-gitlab-com-fails-destination-already-exists-and-is-not-an-empty-directory-error.yml new file mode 100644 index 00000000000..660f4f5d42c --- /dev/null +++ b/changelogs/unreleased/42327-import-from-gitlab-com-fails-destination-already-exists-and-is-not-an-empty-directory-error.yml @@ -0,0 +1,6 @@ +--- +title: Fixes destination already exists, and some particular service errors on Import/Export + error +merge_request: 16714 +author: +type: fixed diff --git a/lib/gitlab/import_export/shared.rb b/lib/gitlab/import_export/shared.rb index d03cbc880fd..b34cafc6876 100644 --- a/lib/gitlab/import_export/shared.rb +++ b/lib/gitlab/import_export/shared.rb @@ -19,8 +19,13 @@ module Gitlab def error(error) error_out(error.message, caller[0].dup) @errors << error.message + # Debug: - Rails.logger.error(error.backtrace.join("\n")) + if error.backtrace + Rails.logger.error("Import/Export backtrace: #{error.backtrace.join("\n")}") + else + Rails.logger.error("No backtrace found") + end end private diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 4cf33778d15..b6c1f0c81cb 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -7096,7 +7096,7 @@ "project_id": 5, "created_at": "2016-06-14T15:01:51.232Z", "updated_at": "2016-06-14T15:01:51.232Z", - "active": false, + "active": true, "properties": { }, diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb index 08e5bbbd400..5804c45871e 100644 --- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb @@ -164,6 +164,10 @@ describe Gitlab::ImportExport::ProjectTreeSaver do expect(saved_project_json['services'].first['type']).to eq('CustomIssueTrackerService') end + it 'saves the properties for a service' do + expect(saved_project_json['services'].first['properties']).to eq('one' => 'value') + end + it 'has project feature' do project_feature = saved_project_json['project_feature'] expect(project_feature).not_to be_empty @@ -279,7 +283,7 @@ describe Gitlab::ImportExport::ProjectTreeSaver do commit_id: ci_build.pipeline.sha) create(:event, :created, target: milestone, project: project, author: user) - create(:service, project: project, type: 'CustomIssueTrackerService', category: 'issue_tracker') + create(:service, project: project, type: 'CustomIssueTrackerService', category: 'issue_tracker', properties: { one: 'value' }) create(:project_custom_attribute, project: project) create(:project_custom_attribute, project: project) diff --git a/spec/workers/repository_import_worker_spec.rb b/spec/workers/repository_import_worker_spec.rb index 7274a9f00f9..2b1a617ee62 100644 --- a/spec/workers/repository_import_worker_spec.rb +++ b/spec/workers/repository_import_worker_spec.rb @@ -49,9 +49,22 @@ describe RepositoryImportWorker do expect do subject.perform(project.id) - end.to raise_error(StandardError, error) + end.to raise_error(RuntimeError, error) expect(project.reload.import_jid).not_to be_nil end + + it 'updates the error on Import/Export' do + error = %q{remote: Not Found fatal: repository 'https://user:pass@test.com/root/repoC.git/' not found } + + project.update_attributes(import_jid: '123', import_type: 'gitlab_project') + expect_any_instance_of(Projects::ImportService).to receive(:execute).and_return({ status: :error, message: error }) + + expect do + subject.perform(project.id) + end.to raise_error(RuntimeError, error) + + expect(project.reload.import_error).not_to be_nil + end end context 'when using an asynchronous importer' do |