Commit cb7a6d34 authored by Stan Hu's avatar Stan Hu

Merge branch...

Merge branch '5794-we-should-failover-gracefully-when-we-can-t-connect-to-geo-tracking-database-ce' into 'master'

Backport: Keep ShaAttribute from halting startup when we can’t connect to a database

See merge request gitlab-org/gitlab-ce!18726
parents 9a2d09e3 18094a17
...@@ -4,18 +4,33 @@ module ShaAttribute ...@@ -4,18 +4,33 @@ module ShaAttribute
module ClassMethods module ClassMethods
def sha_attribute(name) def sha_attribute(name)
return if ENV['STATIC_VERIFICATION'] return if ENV['STATIC_VERIFICATION']
return unless table_exists?
validate_binary_column_exists!(name) unless Rails.env.production?
attribute(name, Gitlab::Database::ShaAttribute.new)
end
# This only gets executed in non-production environments as an additional check to ensure
# the column is the correct type. In production it should behave like any other attribute.
# See https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/5502 for more discussion
def validate_binary_column_exists!(name)
unless table_exists?
warn "WARNING: sha_attribute #{name.inspect} is invalid since the table doesn't exist - you may need to run database migrations"
return
end
column = columns.find { |c| c.name == name.to_s } column = columns.find { |c| c.name == name.to_s }
# In case the table doesn't exist we won't be able to find the column, unless column
# thus we will only check the type if the column is present. raise ArgumentError.new("sha_attribute #{name.inspect} is invalid since the column doesn't exist")
if column && column.type != :binary
raise ArgumentError,
"sha_attribute #{name.inspect} is invalid since the column type is not :binary"
end end
attribute(name, Gitlab::Database::ShaAttribute.new) unless column.type == :binary
raise ArgumentError.new("sha_attribute #{name.inspect} is invalid since the column type is not :binary")
end
rescue => error
Gitlab::AppLogger.error "ShaAttribute initialization: #{error.message}"
raise
end end
end end
end end
---
title: ShaAttribute no longer stops startup if database is missing
merge_request: 18726
author:
type: fixed
...@@ -13,33 +13,74 @@ describe ShaAttribute do ...@@ -13,33 +13,74 @@ describe ShaAttribute do
end end
describe '#sha_attribute' do describe '#sha_attribute' do
context 'when the table exists' do context 'when in non-production' do
before do before do
allow(model).to receive(:table_exists?).and_return(true) allow(Rails.env).to receive(:production?).and_return(false)
end end
it 'defines a SHA attribute for a binary column' do context 'when the table exists' do
expect(model).to receive(:attribute) before do
.with(:sha1, an_instance_of(Gitlab::Database::ShaAttribute)) allow(model).to receive(:table_exists?).and_return(true)
end
model.sha_attribute(:sha1) it 'defines a SHA attribute for a binary column' do
expect(model).to receive(:attribute)
.with(:sha1, an_instance_of(Gitlab::Database::ShaAttribute))
model.sha_attribute(:sha1)
end
it 'raises ArgumentError when the column type is not :binary' do
expect { model.sha_attribute(:name) }.to raise_error(ArgumentError)
end
end
context 'when the table does not exist' do
it 'allows the attribute to be added' do
allow(model).to receive(:table_exists?).and_return(false)
expect(model).not_to receive(:columns)
expect(model).to receive(:attribute)
model.sha_attribute(:name)
end
end end
it 'raises ArgumentError when the column type is not :binary' do context 'when the column does not exist' do
expect { model.sha_attribute(:name) }.to raise_error(ArgumentError) it 'raises ArgumentError' do
allow(model).to receive(:table_exists?).and_return(true)
expect(model).to receive(:columns)
expect(model).not_to receive(:attribute)
expect { model.sha_attribute(:no_name) }.to raise_error(ArgumentError)
end
end
context 'when other execeptions are raised' do
it 'logs and re-rasises the error' do
allow(model).to receive(:table_exists?).and_raise(ActiveRecord::NoDatabaseError.new('does not exist'))
expect(model).not_to receive(:columns)
expect(model).not_to receive(:attribute)
expect(Gitlab::AppLogger).to receive(:error)
expect { model.sha_attribute(:name) }.to raise_error(ActiveRecord::NoDatabaseError)
end
end end
end end
context 'when the table does not exist' do context 'when in production' do
before do before do
allow(model).to receive(:table_exists?).and_return(false) allow(Rails.env).to receive(:production?).and_return(true)
end end
it 'does nothing' do it 'defines a SHA attribute' do
expect(model).not_to receive(:table_exists?)
expect(model).not_to receive(:columns) expect(model).not_to receive(:columns)
expect(model).not_to receive(:attribute) expect(model).to receive(:attribute).with(:sha1, an_instance_of(Gitlab::Database::ShaAttribute))
model.sha_attribute(:name) model.sha_attribute(:sha1)
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