Commit 3710b9a8 authored by Mike Kozono's avatar Mike Kozono

Move raising and logging NotImplementedError

...to `Replicator.for_replicable_name`, since we would want to
know anytime this happens.
parent 7bf9aef1
...@@ -3,7 +3,6 @@ ...@@ -3,7 +3,6 @@
module Geo module Geo
class BlobUploadService class BlobUploadService
include ExclusiveLeaseGuard include ExclusiveLeaseGuard
include ::Gitlab::Geo::LogHelpers
attr_reader :replicable_name, :blob_id, :checksum, :replicator attr_reader :replicable_name, :blob_id, :checksum, :replicator
...@@ -14,8 +13,6 @@ module Geo ...@@ -14,8 +13,6 @@ module Geo
replicator_klass = Gitlab::Geo::Replicator.for_replicable_name(replicable_name) replicator_klass = Gitlab::Geo::Replicator.for_replicable_name(replicable_name)
@replicator = replicator_klass.new(model_record_id: blob_id) @replicator = replicator_klass.new(model_record_id: blob_id)
fail_unimplemented!(replicable_name) unless @replicator
end end
def execute def execute
...@@ -25,25 +22,5 @@ module Geo ...@@ -25,25 +22,5 @@ module Geo
def retriever def retriever
Gitlab::Geo::Replication::BlobRetriever.new(replicator: replicator, checksum: checksum) Gitlab::Geo::Replication::BlobRetriever.new(replicator: replicator, checksum: checksum)
end end
private
def fail_unimplemented!(replicable_name)
error_message = "Cannot find a Geo::Replicator for #{replicable_name}"
log_error(error_message)
raise NotImplementedError, error_message
end
# This is called by LogHelpers to build json log with context info
#
# @see ::Gitlab::Geo::LogHelpers
def extra_log_data
{
replicable_name: replicable_name,
blob_id: blob_id
}.compact
end
end end
end end
...@@ -11,6 +11,7 @@ module Gitlab ...@@ -11,6 +11,7 @@ module Gitlab
# Each replicator is tied to a specific replicable resource # Each replicator is tied to a specific replicable resource
class Replicator class Replicator
include ::Gitlab::Geo::LogHelpers include ::Gitlab::Geo::LogHelpers
extend ::Gitlab::Geo::LogHelpers
CLASS_SUFFIXES = %w(RegistryFinder RegistriesResolver).freeze CLASS_SUFFIXES = %w(RegistryFinder RegistriesResolver).freeze
...@@ -73,7 +74,7 @@ module Gitlab ...@@ -73,7 +74,7 @@ module Gitlab
const_get("::Types::Geo::#{replicable_name.camelize}RegistryType", false) const_get("::Types::Geo::#{replicable_name.camelize}RegistryType", false)
end end
# Given a `replicable_name`, return the corresponding replicator # Given a `replicable_name`, return the corresponding replicator class
# #
# @param [String] replicable_name the replicable slug # @param [String] replicable_name the replicable slug
# @return [Class<Geo::Replicator>] replicator implementation # @return [Class<Geo::Replicator>] replicator implementation
...@@ -81,6 +82,13 @@ module Gitlab ...@@ -81,6 +82,13 @@ module Gitlab
replicator_class_name = "::Geo::#{replicable_name.camelize}Replicator" replicator_class_name = "::Geo::#{replicable_name.camelize}Replicator"
const_get(replicator_class_name, false) const_get(replicator_class_name, false)
rescue NameError
message = "Cannot find a Geo::Replicator for #{replicable_name}"
e = NotImplementedError.new(message)
log_error(message, e, { replicable_name: replicable_name })
raise e
end end
def self.checksummed def self.checksummed
......
...@@ -146,5 +146,33 @@ describe Gitlab::Geo::Replicator do ...@@ -146,5 +146,33 @@ describe Gitlab::Geo::Replicator do
end end
end end
end end
describe '.for_replicable_name' do
context 'given a valid replicable_name' do
it 'returns the corresponding Replicator class' do
replicator_class = described_class.for_replicable_name('dummy')
expect(replicator_class).to eq(Geo::DummyReplicator)
end
end
context 'given an invalid replicable_name' do
it 'raises and logs NotImplementedError' do
expect(Gitlab::Geo::Logger).to receive(:error)
expect do
described_class.for_replicable_name('invalid')
end.to raise_error(NotImplementedError)
end
end
context 'given nil' do
it 'raises NotImplementedError' do
expect do
described_class.for_replicable_name('invalid')
end.to raise_error(NotImplementedError)
end
end
end
end end
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