summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2018-07-09 10:02:03 +0000
committerDouwe Maan <douwe@gitlab.com>2018-07-09 10:02:03 +0000
commita153925574173ef5d6494ecd14858fc48b2c3525 (patch)
treeb9331dbb9018f6a42dc7ca2014e5524b94b6449e
parent090f4d9e9f409c76bd5a8738a1b39b1366845590 (diff)
parent3082b7d1c2524845441a529fd0704607cf2e8cb1 (diff)
downloadgitlab-ce-a153925574173ef5d6494ecd14858fc48b2c3525.tar.gz
Merge branch 'gitaly-mandatory-20180704-jv' into 'master'
Make blob and other RPC's mandatory Closes gitaly#331, gitaly#384, gitaly#446, and gitaly#383 See merge request gitlab-org/gitlab-ce!20378
-rw-r--r--lib/gitlab/git/blob.rb19
-rw-r--r--lib/gitlab/git/repository.rb116
-rw-r--r--lib/gitlab/workhorse.rb23
-rw-r--r--spec/lib/gitlab/git/blob_spec.rb7
-rw-r--r--spec/lib/gitlab/workhorse_spec.rb42
5 files changed, 36 insertions, 171 deletions
diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb
index 604bb11e712..96fa94d5790 100644
--- a/lib/gitlab/git/blob.rb
+++ b/lib/gitlab/git/blob.rb
@@ -50,13 +50,7 @@ module Gitlab
end
def raw(repository, sha)
- Gitlab::GitalyClient.migrate(:git_blob_raw) do |is_enabled|
- if is_enabled
- repository.gitaly_blob_client.get_blob(oid: sha, limit: MAX_DATA_DISPLAY_SIZE)
- else
- rugged_raw(repository, sha, limit: MAX_DATA_DISPLAY_SIZE)
- end
- end
+ repository.gitaly_blob_client.get_blob(oid: sha, limit: MAX_DATA_DISPLAY_SIZE)
end
# Returns an array of Blob instances, specified in blob_references as
@@ -207,16 +201,7 @@ module Gitlab
return if @loaded_all_data
- @data = Gitlab::GitalyClient.migrate(:git_blob_load_all_data) do |is_enabled|
- begin
- if is_enabled
- repository.gitaly_blob_client.get_blob(oid: id, limit: -1).data
- else
- repository.lookup(id).content
- end
- end
- end
-
+ @data = repository.gitaly_blob_client.get_blob(oid: id, limit: -1).data
@loaded_all_data = true
@loaded_size = @data.bytesize
end
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index b355e5e87ff..420790f45d0 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -439,31 +439,11 @@ module Gitlab
raise ArgumentError.new("invalid Repository#log limit: #{limit.inspect}")
end
- gitaly_migrate(:find_commits) do |is_enabled|
- if is_enabled
- gitaly_commit_client.find_commits(options)
- else
- raw_log(options).map { |c| Commit.decorate(self, c) }
- end
+ wrapped_gitaly_errors do
+ gitaly_commit_client.find_commits(options)
end
end
- # Used in gitaly-ruby
- def raw_log(options)
- sha =
- unless options[:all]
- actual_ref = options[:ref] || root_ref
- begin
- sha_from_ref(actual_ref)
- rescue Rugged::OdbError, Rugged::InvalidError, Rugged::ReferenceError
- # Return an empty array if the ref wasn't found
- return []
- end
- end
-
- log_by_shell(sha, options)
- end
-
def count_commits(options)
options = process_count_commits_options(options.dup)
@@ -1223,12 +1203,10 @@ module Gitlab
end
def find_commits_by_message(query, ref, path, limit, offset)
- gitaly_migrate(:commits_by_message) do |is_enabled|
- if is_enabled
- find_commits_by_message_by_gitaly(query, ref, path, limit, offset)
- else
- find_commits_by_message_by_shelling_out(query, ref, path, limit, offset)
- end
+ wrapped_gitaly_errors do
+ gitaly_commit_client
+ .commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset)
+ .map { |c| commit(c) }
end
end
@@ -1238,12 +1216,8 @@ module Gitlab
end
def last_commit_for_path(sha, path)
- gitaly_migrate(:last_commit_for_path) do |is_enabled|
- if is_enabled
- last_commit_for_path_by_gitaly(sha, path)
- else
- last_commit_for_path_by_rugged(sha, path)
- end
+ wrapped_gitaly_errors do
+ gitaly_commit_client.last_commit_for_path(sha, path)
end
end
@@ -1454,46 +1428,6 @@ module Gitlab
end
end
- # Gitaly note: JV: although #log_by_shell shells out to Git I think the
- # complexity is such that we should migrate it as Ruby before trying to
- # do it in Go.
- def log_by_shell(sha, options)
- limit = options[:limit].to_i
- offset = options[:offset].to_i
- use_follow_flag = options[:follow] && options[:path].present?
-
- # We will perform the offset in Ruby because --follow doesn't play well with --skip.
- # See: https://gitlab.com/gitlab-org/gitlab-ce/issues/3574#note_3040520
- offset_in_ruby = use_follow_flag && options[:offset].present?
- limit += offset if offset_in_ruby
-
- cmd = %w[log]
- cmd << "--max-count=#{limit}"
- cmd << '--format=%H'
- cmd << "--skip=#{offset}" unless offset_in_ruby
- cmd << '--follow' if use_follow_flag
- cmd << '--no-merges' if options[:skip_merges]
- cmd << "--after=#{options[:after].iso8601}" if options[:after]
- cmd << "--before=#{options[:before].iso8601}" if options[:before]
-
- if options[:all]
- cmd += %w[--all --reverse]
- else
- cmd << sha
- end
-
- # :path can be a string or an array of strings
- if options[:path].present?
- cmd << '--'
- cmd += Array(options[:path])
- end
-
- raw_output, _status = run_git(cmd)
- lines = offset_in_ruby ? raw_output.lines.drop(offset) : raw_output.lines
-
- lines.map! { |c| Rugged::Commit.new(rugged, c.strip) }
- end
-
# We are trying to deprecate this method because it does a lot of work
# but it seems to be used only to look up submodule URL's.
# https://gitlab.com/gitlab-org/gitaly/issues/329
@@ -1623,11 +1557,6 @@ module Gitlab
end
end
- def last_commit_for_path_by_rugged(sha, path)
- sha = last_commit_id_for_path_by_shelling_out(sha, path)
- commit(sha)
- end
-
# Returns true if the given ref name exists
#
# Ref names must start with `refs/`.
@@ -1820,35 +1749,6 @@ module Gitlab
raise CommandError, @gitlab_projects.output
end
- def find_commits_by_message_by_shelling_out(query, ref, path, limit, offset)
- ref ||= root_ref
-
- args = %W(
- log #{ref} --pretty=%H --skip #{offset}
- --max-count #{limit} --grep=#{query} --regexp-ignore-case
- )
- args = args.concat(%W(-- #{path})) if path.present?
-
- git_log_results = run_git(args).first.lines
-
- git_log_results.map { |c| commit(c.chomp) }.compact
- end
-
- def find_commits_by_message_by_gitaly(query, ref, path, limit, offset)
- gitaly_commit_client
- .commits_by_message(query, revision: ref, path: path, limit: limit, offset: offset)
- .map { |c| commit(c) }
- end
-
- def last_commit_for_path_by_gitaly(sha, path)
- gitaly_commit_client.last_commit_for_path(sha, path)
- end
-
- def last_commit_id_for_path_by_shelling_out(sha, path)
- args = %W(rev-list --max-count=1 #{sha} -- #{path})
- run_git_with_timeout(args, Gitlab::Git::Popen::FAST_GIT_PROCESS_TIMEOUT).first.strip
- end
-
def rugged_merge_base(from, to)
rugged.merge_base(from, to)
rescue Rugged::ReferenceError
diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb
index 79483ee05f8..55c899912f9 100644
--- a/lib/gitlab/workhorse.rb
+++ b/lib/gitlab/workhorse.rb
@@ -37,21 +37,14 @@ module Gitlab
end
def send_git_blob(repository, blob)
- params = if Gitlab::GitalyClient.feature_enabled?(:workhorse_raw_show, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT)
- {
- 'GitalyServer' => gitaly_server_hash(repository),
- 'GetBlobRequest' => {
- repository: repository.gitaly_repository.to_h,
- oid: blob.id,
- limit: -1
- }
- }
- else
- {
- 'RepoPath' => repository.path_to_repo,
- 'BlobId' => blob.id
- }
- end
+ params = {
+ 'GitalyServer' => gitaly_server_hash(repository),
+ 'GetBlobRequest' => {
+ repository: repository.gitaly_repository.to_h,
+ oid: blob.id,
+ limit: -1
+ }
+ }
[
SEND_DATA_HEADER,
diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb
index b6061df349d..6900c4189b7 100644
--- a/spec/lib/gitlab/git/blob_spec.rb
+++ b/spec/lib/gitlab/git/blob_spec.rb
@@ -532,8 +532,8 @@ describe Gitlab::Git::Blob, seed_helper: true do
subject { blob.load_all_data!(repository) }
it 'loads missing data' do
- expect(Gitlab::GitalyClient).to receive(:migrate)
- .with(:git_blob_load_all_data).and_return(full_data)
+ expect(repository.gitaly_blob_client).to receive(:get_blob)
+ .and_return(double(:response, data: full_data))
subject
@@ -544,8 +544,7 @@ describe Gitlab::Git::Blob, seed_helper: true do
let(:blob) { Gitlab::Git::Blob.new(name: 'test', size: 4, data: full_data) }
it "doesn't perform any loading" do
- expect(Gitlab::GitalyClient).not_to receive(:migrate)
- .with(:git_blob_load_all_data)
+ expect(repository.gitaly_blob_client).not_to receive(:get_blob)
subject
diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb
index 570580e0cfc..8fdcfe79fb5 100644
--- a/spec/lib/gitlab/workhorse_spec.rb
+++ b/spec/lib/gitlab/workhorse_spec.rb
@@ -413,34 +413,22 @@ describe Gitlab::Workhorse do
subject { described_class.send_git_blob(repository, blob) }
- context 'when Gitaly workhorse_raw_show feature is enabled' do
- it 'sets the header correctly' do
- key, command, params = decode_workhorse_header(subject)
-
- expect(key).to eq('Gitlab-Workhorse-Send-Data')
- expect(command).to eq('git-blob')
- expect(params).to eq({
- 'GitalyServer' => {
- address: Gitlab::GitalyClient.address(project.repository_storage),
- token: Gitlab::GitalyClient.token(project.repository_storage)
- },
- 'GetBlobRequest' => {
- repository: repository.gitaly_repository.to_h,
- oid: blob.id,
- limit: -1
- }
- }.deep_stringify_keys)
- end
- end
-
- context 'when Gitaly workhorse_raw_show feature is disabled', :disable_gitaly do
- it 'sets the header correctly' do
- key, command, params = decode_workhorse_header(subject)
+ it 'sets the header correctly' do
+ key, command, params = decode_workhorse_header(subject)
- expect(key).to eq('Gitlab-Workhorse-Send-Data')
- expect(command).to eq('git-blob')
- expect(params).to eq('RepoPath' => repository.path_to_repo, 'BlobId' => blob.id)
- end
+ expect(key).to eq('Gitlab-Workhorse-Send-Data')
+ expect(command).to eq('git-blob')
+ expect(params).to eq({
+ 'GitalyServer' => {
+ address: Gitlab::GitalyClient.address(project.repository_storage),
+ token: Gitlab::GitalyClient.token(project.repository_storage)
+ },
+ 'GetBlobRequest' => {
+ repository: repository.gitaly_repository.to_h,
+ oid: blob.id,
+ limit: -1
+ }
+ }.deep_stringify_keys)
end
end