Skip to content

Commit

Permalink
Recover from four kinds of init error
Browse files Browse the repository at this point in the history
- ActiveRecord::SchemaMigrations and internal_metadata tables are being considered for dfe-analytics (ignore them)
- discovering migrations pending on dfe-analytics init no longer bails
  pre-emptively
- discovering missing database connection when database not present,
  also on init, also no longer bails pre-emptively
- don't eager load unless we've got a database connection

The final example is important because some contexts which lack db
connections (eg asset precompilation) also don't provide all necessary
eg ENV vars which would be required were we loading the whole app (which
this init process does).

So discover early if we have no database (call
ActiveRecord::Base.connection). & if anything happens during
Fields.check! catch that too. Log lines will tell us if we're bailing
for want of a DB or for want of a migration. (There's no point in
running check! if migration yet to run).
  • Loading branch information
duncanjbrown committed Sep 29, 2022
1 parent 2e39358 commit 4e2e86b
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 9 deletions.
19 changes: 10 additions & 9 deletions lib/dfe/analytics.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,7 @@ def self.configure
end

def self.initialize!
begin
ActiveRecord::Base.connection
rescue ActiveRecord::ActiveRecordError
Rails.logger.info('No database connection; DfE Analytics not initialized')
return
end

ActiveRecord::Base.connection # cause an exception early if we can't connect
DfE::Analytics::Fields.check!

entities_for_analytics.each do |entity|
Expand All @@ -96,6 +90,10 @@ def self.initialize!
model.include(DfE::Analytics::Entities)
end
end
rescue ActiveRecord::PendingMigrationError
Rails.logger.info('Database requires migration; DfE Analytics not initialized')
rescue ActiveRecord::ActiveRecordError
Rails.logger.info('No database connection; DfE Analytics not initialized')
end

def self.enabled?
Expand Down Expand Up @@ -177,9 +175,12 @@ def self.entity_model_mapping

Rails.application.eager_load!

rails_tables = %w[ar_internal_metadata schema_migrations]

ActiveRecord::Base.descendants
.reject(&:abstract_class?)
.index_by(&:table_name)
.reject(&:abstract_class?)
.index_by(&:table_name)
.except(*rails_tables)
end
end

Expand Down
9 changes: 9 additions & 0 deletions spec/dfe/analytics_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,15 @@
end
end

describe 'when migrations are pending' do
it 'recovers and logs' do
allow(DfE::Analytics::Fields).to receive(:check!).and_raise(ActiveRecord::PendingMigrationError)

allow(Rails.logger).to receive(:info).with(/Database requires migration/)
expect { DfE::Analytics.initialize! }.not_to raise_error
end
end

describe 'field checks on initialization' do
# field validity is computed from allowlist, blocklist and database. See
# Analytics::Fields for more details
Expand Down

0 comments on commit 4e2e86b

Please sign in to comment.