MigrateProcessCommitWorkerJobs to use Gitaly
This old migration used Rugged to find a commit, while Gitaly is the prefered way now. By migrating this to Gitaly, Gitaly is now a required running component for this migration. Part of https://gitlab.com/gitlab-org/gitaly/issues/1106
This commit is contained in:
parent
928c81e259
commit
cfbb256b8b
|
@ -1,9 +1,19 @@
|
||||||
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
|
|
||||||
# for more information on how to write migrations for GitLab.
|
|
||||||
|
|
||||||
class MigrateProcessCommitWorkerJobs < ActiveRecord::Migration
|
class MigrateProcessCommitWorkerJobs < ActiveRecord::Migration
|
||||||
include Gitlab::Database::MigrationHelpers
|
include Gitlab::Database::MigrationHelpers
|
||||||
|
|
||||||
|
class Repository
|
||||||
|
attr_reader :storage
|
||||||
|
|
||||||
|
def initialize(storage, relative_path)
|
||||||
|
@storage = storage
|
||||||
|
@relative_path = relative_path
|
||||||
|
end
|
||||||
|
|
||||||
|
def gitaly_repository
|
||||||
|
Gitaly::Repository.new(storage_name: @storage, relative_path: @relative_path)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
class Project < ActiveRecord::Base
|
class Project < ActiveRecord::Base
|
||||||
def self.find_including_path(id)
|
def self.find_including_path(id)
|
||||||
select("projects.*, CONCAT(namespaces.path, '/', projects.path) AS path_with_namespace")
|
select("projects.*, CONCAT(namespaces.path, '/', projects.path) AS path_with_namespace")
|
||||||
|
@ -11,19 +21,12 @@ class MigrateProcessCommitWorkerJobs < ActiveRecord::Migration
|
||||||
.find_by(id: id)
|
.find_by(id: id)
|
||||||
end
|
end
|
||||||
|
|
||||||
def repository_storage_path
|
def commit(rev)
|
||||||
Gitlab::GitalyClient::StorageSettings.allow_disk_access do
|
Gitlab::GitalyClient::CommitService.new(repository).find_commit(rev)
|
||||||
Gitlab.config.repositories.storages[repository_storage].legacy_disk_path
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
def repository_path
|
|
||||||
# TODO: review if the change from Legacy storage needs to reflect here as well.
|
|
||||||
File.join(repository_storage_path, read_attribute(:path_with_namespace) + '.git')
|
|
||||||
end
|
end
|
||||||
|
|
||||||
def repository
|
def repository
|
||||||
@repository ||= Rugged::Repository.new(repository_path)
|
@repository ||= Repository.new(repository_storage, read_attribute(:path_with_namespace) + '.git')
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -42,22 +45,19 @@ class MigrateProcessCommitWorkerJobs < ActiveRecord::Migration
|
||||||
|
|
||||||
next unless project
|
next unless project
|
||||||
|
|
||||||
begin
|
commit = project.commit(payload['args'][2])
|
||||||
commit = project.repository.lookup(payload['args'][2])
|
next unless commit
|
||||||
rescue Rugged::OdbError
|
|
||||||
next
|
|
||||||
end
|
|
||||||
|
|
||||||
hash = {
|
hash = {
|
||||||
id: commit.oid,
|
id: commit.id,
|
||||||
message: encode(commit.message),
|
message: encode(commit.body),
|
||||||
parent_ids: commit.parent_ids,
|
parent_ids: commit.parent_ids.to_a,
|
||||||
authored_date: commit.author[:time],
|
authored_date: Time.at(commit.author.date.seconds).utc,
|
||||||
author_name: encode(commit.author[:name]),
|
author_name: encode(commit.author.name),
|
||||||
author_email: encode(commit.author[:email]),
|
author_email: encode(commit.author.email),
|
||||||
committed_date: commit.committer[:time],
|
committed_date: Time.at(commit.committer.date.seconds).utc,
|
||||||
committer_email: encode(commit.committer[:email]),
|
committer_email: encode(commit.committer.email),
|
||||||
committer_name: encode(commit.committer[:name])
|
committer_name: encode(commit.committer.name)
|
||||||
}
|
}
|
||||||
|
|
||||||
payload['args'][2] = hash
|
payload['args'][2] = hash
|
||||||
|
|
|
@ -1,4 +1,3 @@
|
||||||
# Gitlab::Git::Repository is a wrapper around native Rugged::Repository object
|
|
||||||
require 'tempfile'
|
require 'tempfile'
|
||||||
require 'forwardable'
|
require 'forwardable'
|
||||||
require "rubygems/package"
|
require "rubygems/package"
|
||||||
|
|
|
@ -4,14 +4,11 @@ require 'spec_helper'
|
||||||
require Rails.root.join('db', 'migrate', '20161124141322_migrate_process_commit_worker_jobs.rb')
|
require Rails.root.join('db', 'migrate', '20161124141322_migrate_process_commit_worker_jobs.rb')
|
||||||
|
|
||||||
describe MigrateProcessCommitWorkerJobs do
|
describe MigrateProcessCommitWorkerJobs do
|
||||||
let(:project) { create(:project, :legacy_storage, :repository) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
|
set(:project) { create(:project, :legacy_storage, :repository) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
|
||||||
let(:user) { create(:user) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
|
set(:user) { create(:user) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
|
||||||
let(:rugged) do
|
let(:commit) do
|
||||||
Gitlab::GitalyClient::StorageSettings.allow_disk_access do
|
Gitlab::Git::Commit.last(project.repository.raw)
|
||||||
project.repository.rugged
|
|
||||||
end
|
end
|
||||||
end
|
|
||||||
let(:commit) { rugged.rev_parse(project.commit.id) }
|
|
||||||
|
|
||||||
describe 'Project' do
|
describe 'Project' do
|
||||||
describe 'find_including_path' do
|
describe 'find_including_path' do
|
||||||
|
@ -29,32 +26,13 @@ describe MigrateProcessCommitWorkerJobs do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#repository_storage_path' do
|
|
||||||
it 'returns the storage path for the repository' do
|
|
||||||
migration_project = described_class::Project
|
|
||||||
.find_including_path(project.id)
|
|
||||||
|
|
||||||
expect(File.directory?(migration_project.repository_storage_path))
|
|
||||||
.to eq(true)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
describe '#repository_path' do
|
|
||||||
it 'returns the path to the repository' do
|
|
||||||
migration_project = described_class::Project
|
|
||||||
.find_including_path(project.id)
|
|
||||||
|
|
||||||
expect(File.directory?(migration_project.repository_path)).to eq(true)
|
|
||||||
end
|
|
||||||
end
|
|
||||||
|
|
||||||
describe '#repository' do
|
describe '#repository' do
|
||||||
it 'returns a Rugged::Repository' do
|
it 'returns a Rugged::Repository' do
|
||||||
migration_project = described_class::Project
|
migration_project = described_class::Project
|
||||||
.find_including_path(project.id)
|
.find_including_path(project.id)
|
||||||
|
|
||||||
expect(migration_project.repository)
|
expect(migration_project.repository).to respond_to(:storage)
|
||||||
.to be_an_instance_of(Rugged::Repository)
|
expect(migration_project.repository).to respond_to(:gitaly_repository)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -72,7 +50,7 @@ describe MigrateProcessCommitWorkerJobs do
|
||||||
|
|
||||||
before do
|
before do
|
||||||
Sidekiq.redis do |redis|
|
Sidekiq.redis do |redis|
|
||||||
job = JSON.dump(args: [project.id, user.id, commit.oid])
|
job = JSON.dump(args: [project.id, user.id, commit.id])
|
||||||
redis.lpush('queue:process_commit', job)
|
redis.lpush('queue:process_commit', job)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -88,9 +66,10 @@ describe MigrateProcessCommitWorkerJobs do
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'skips jobs using commits that no longer exist' do
|
it 'skips jobs using commits that no longer exist' do
|
||||||
allow_any_instance_of(Rugged::Repository).to receive(:lookup)
|
allow_any_instance_of(Gitlab::GitalyClient::CommitService)
|
||||||
.with(commit.oid)
|
.to receive(:find_commit)
|
||||||
.and_raise(Rugged::OdbError)
|
.with(commit.id)
|
||||||
|
.and_return(nil)
|
||||||
|
|
||||||
migration.up
|
migration.up
|
||||||
|
|
||||||
|
@ -105,7 +84,7 @@ describe MigrateProcessCommitWorkerJobs do
|
||||||
|
|
||||||
it 'encodes data to UTF-8' do
|
it 'encodes data to UTF-8' do
|
||||||
allow_any_instance_of(Rugged::Repository).to receive(:lookup)
|
allow_any_instance_of(Rugged::Repository).to receive(:lookup)
|
||||||
.with(commit.oid)
|
.with(commit.id)
|
||||||
.and_return(commit)
|
.and_return(commit)
|
||||||
|
|
||||||
allow(commit).to receive(:message)
|
allow(commit).to receive(:message)
|
||||||
|
@ -140,7 +119,7 @@ describe MigrateProcessCommitWorkerJobs do
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'includes the commit ID' do
|
it 'includes the commit ID' do
|
||||||
expect(commit_hash['id']).to eq(commit.oid)
|
expect(commit_hash['id']).to eq(commit.id)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'includes the commit message' do
|
it 'includes the commit message' do
|
||||||
|
@ -152,27 +131,27 @@ describe MigrateProcessCommitWorkerJobs do
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'includes the author date' do
|
it 'includes the author date' do
|
||||||
expect(commit_hash['authored_date']).to eq(commit.author[:time].to_s)
|
expect(commit_hash['authored_date']).to eq(commit.authored_date.to_s)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'includes the author name' do
|
it 'includes the author name' do
|
||||||
expect(commit_hash['author_name']).to eq(commit.author[:name])
|
expect(commit_hash['author_name']).to eq(commit.author_name)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'includes the author Email' do
|
it 'includes the author Email' do
|
||||||
expect(commit_hash['author_email']).to eq(commit.author[:email])
|
expect(commit_hash['author_email']).to eq(commit.author_email)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'includes the commit date' do
|
it 'includes the commit date' do
|
||||||
expect(commit_hash['committed_date']).to eq(commit.committer[:time].to_s)
|
expect(commit_hash['committed_date']).to eq(commit.committed_date.to_s)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'includes the committer name' do
|
it 'includes the committer name' do
|
||||||
expect(commit_hash['committer_name']).to eq(commit.committer[:name])
|
expect(commit_hash['committer_name']).to eq(commit.committer_name)
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'includes the committer Email' do
|
it 'includes the committer Email' do
|
||||||
expect(commit_hash['committer_email']).to eq(commit.committer[:email])
|
expect(commit_hash['committer_email']).to eq(commit.committer_email)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
@ -186,7 +165,7 @@ describe MigrateProcessCommitWorkerJobs do
|
||||||
|
|
||||||
before do
|
before do
|
||||||
Sidekiq.redis do |redis|
|
Sidekiq.redis do |redis|
|
||||||
job = JSON.dump(args: [project.id, user.id, commit.oid])
|
job = JSON.dump(args: [project.id, user.id, commit.id])
|
||||||
redis.lpush('queue:process_commit', job)
|
redis.lpush('queue:process_commit', job)
|
||||||
|
|
||||||
migration.up
|
migration.up
|
||||||
|
@ -215,7 +194,7 @@ describe MigrateProcessCommitWorkerJobs do
|
||||||
end
|
end
|
||||||
|
|
||||||
it 'includes the commit SHA' do
|
it 'includes the commit SHA' do
|
||||||
expect(job['args'][2]).to eq(commit.oid)
|
expect(job['args'][2]).to eq(commit.id)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue