Skip to content

Commit

Permalink
Merge pull request rails#50463 from Shopify/remove-sqlite3-production…
Browse files Browse the repository at this point in the history
…-warning

Remove SQLite production warning but leave production config disabled
  • Loading branch information
byroot authored Dec 27, 2023
2 parents e7f2db6 + 6b446be commit 354d1c4
Show file tree
Hide file tree
Showing 7 changed files with 25 additions and 83 deletions.
1 change: 1 addition & 0 deletions .github/workflows/rails-new-docker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ jobs:
podman run --name $APP_NAME \
-v $(pwd):$(pwd) \
-e SECRET_KEY_BASE_DUMMY=1 \
-e DATABASE_URL=sqlite3:storage/production.sqlite3 \
-p 3000:3000 $APP_NAME &
- name: Test container
run: ruby -r ./.github/workflows/scripts/test-container.rb
Expand Down
9 changes: 9 additions & 0 deletions activerecord/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,12 @@
* Remove warning message when running SQLite in production, but leave it unconfigured

There are valid use cases for running SQLite in production, however it must be done
with care, so instead of a warning most users won't see anyway, it's preferable to
leave the configuration commented out to force them to think about having the database
on a persistent volume etc.

*Jacopo Beschi*, *Jean Boussier*

* Add support for generated columns in SQLite3 adapter

Generated columns (both stored and dynamic) are supported since version 3.31.0 of SQLite.
Expand Down
16 changes: 7 additions & 9 deletions activerecord/lib/active_record/railtie.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ class Railtie < Rails::Railtie # :nodoc:
config.active_record.check_schema_cache_dump_version = true
config.active_record.maintain_test_schema = true
config.active_record.has_many_inversing = false
config.active_record.sqlite3_production_warning = true
config.active_record.query_log_tags_enabled = false
config.active_record.query_log_tags = [ :application ]
config.active_record.query_log_tags_format = :legacy
Expand Down Expand Up @@ -232,13 +231,13 @@ class Railtie < Rails::Railtie # :nodoc:
end
end

SQLITE3_PRODUCTION_WARN = "You are running SQLite in production, this is generally not recommended."\
" You can disable this warning by setting \"config.active_record.sqlite3_production_warning=false\"."
initializer "active_record.sqlite3_production_warning" do
if config.active_record.sqlite3_production_warning && Rails.env.production?
ActiveSupport.on_load(:active_record_sqlite3adapter) do
Rails.logger.warn(SQLITE3_PRODUCTION_WARN)
end
initializer "active_record.sqlite3_deprecated_warning" do
if config.active_record.key?(:sqlite3_production_warning)
config.active_record.delete(:sqlite3_production_warning)
ActiveRecord.deprecator.warn <<~MSG.squish
The `config.active_record.sqlite3_production_warning` configuration no longer has any effect
and can be safely removed.
MSG
end
end

Expand Down Expand Up @@ -278,7 +277,6 @@ class Railtie < Rails::Railtie # :nodoc:
:query_log_tags,
:query_log_tags_format,
:cache_query_log_tags,
:sqlite3_production_warning,
:sqlite3_adapter_strict_strings_by_default,
:check_schema_cache_dump_version,
:use_schema_cache_dump
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ test:
<<: *default
database: storage/test.sqlite3


# SQLite3 write its data on the local filesystem, as such it requires
# persistent disks. If you are deploying to a managed service, you should
# make sure it provides disk persistence, as many don't.
#
# Similarly, if you deploy your application as a Docker container, you must
# ensure the database is located in a persisted volume.
production:
<<: *default
database: storage/production.sqlite3
# database: path/to/persistent/storage/production.sqlite3
71 changes: 0 additions & 71 deletions railties/test/application/configuration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,6 @@ def self.safe_list_sanitizer
end
end

class MyLogRecorder < Logger
def initialize
@io = StringIO.new
super(@io)
end

def recording
@io.string
end
end

module ApplicationTests
class ConfigurationTest < ActiveSupport::TestCase
include ActiveSupport::Testing::Isolation
Expand Down Expand Up @@ -79,7 +68,6 @@ def switch_development_hosts_to(*hosts)
def setup
build_app
suppress_default_config
suppress_sqlite3_warning
end

def teardown
Expand All @@ -96,14 +84,6 @@ def restore_default_config
FileUtils.mv("#{app_path}/config/__environments__", "#{app_path}/config/environments")
end

def suppress_sqlite3_warning
add_to_config "config.active_record.sqlite3_production_warning = false"
end

def restore_sqlite3_warning
remove_from_config ".*config.active_record.sqlite3_production_warning.*\n"
end

test "Rails.env does not set the RAILS_ENV environment variable which would leak out into rake tasks" do
require "rails"

Expand Down Expand Up @@ -4061,57 +4041,6 @@ class Post < ActiveRecord::Base
assert_equal false, Rails.application.env_config["action_dispatch.log_rescued_responses"]
end

test "logs a warning when running SQLite3 in production" do
restore_sqlite3_warning
app_file "config/initializers/active_record.rb", <<~RUBY
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
RUBY
add_to_config "config.logger = MyLogRecorder.new"

app "production"

assert_match(/You are running SQLite in production, this is generally not recommended/, Rails.logger.recording)
end

test "doesn't log a warning when running SQLite3 in production and sqlite3_production_warning=false" do
app_file "config/initializers/active_record.rb", <<~RUBY
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
RUBY
add_to_config "config.logger = MyLogRecorder.new"

app "production"

assert_no_match(/You are running SQLite in production, this is generally not recommended/, Rails.logger.recording)
end

test "doesn't log a warning when running MySQL in production" do
restore_sqlite3_warning
original_configurations = ActiveRecord::Base.configurations
ActiveRecord::Base.configurations = { production: { db1: { adapter: "mysql2" } } }
app_file "config/initializers/active_record.rb", <<~RUBY
ActiveRecord::Base.establish_connection(adapter: "mysql2")
RUBY
add_to_config "config.logger = MyLogRecorder.new"

app "production"

assert_no_match(/You are running SQLite in production, this is generally not recommended/, Rails.logger.recording)
ensure
ActiveRecord::Base.configurations = original_configurations
end

test "doesn't log a warning when running SQLite3 in development" do
restore_sqlite3_warning
app_file "config/initializers/active_record.rb", <<~RUBY
ActiveRecord::Base.establish_connection(adapter: "sqlite3", database: ":memory:")
RUBY
add_to_config "config.logger = MyLogRecorder.new"

app "development"

assert_no_match(/You are running SQLite in production, this is generally not recommended/, Rails.logger.recording)
end

test "app starts with LocalCache middleware" do
app "development"

Expand Down
1 change: 0 additions & 1 deletion railties/test/application/loading_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,6 @@ def test_initialize_can_be_called_at_any_time
end

test "active record query cache hooks are installed before first request in production" do
add_to_config "config.active_record.sqlite3_production_warning = false"
app_file "app/controllers/omg_controller.rb", <<-RUBY
begin
class OmgController < ActionController::Metal
Expand Down
1 change: 0 additions & 1 deletion railties/test/application/query_logs_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ class QueryLogsTest < ActiveSupport::TestCase

def setup
build_app(multi_db: true)
add_to_config "config.active_record.sqlite3_production_warning = false"
rails("generate", "scaffold", "Pet", "name:string", "--database=animals")
app_file "app/models/user.rb", <<-RUBY
class User < ActiveRecord::Base
Expand Down

0 comments on commit 354d1c4

Please sign in to comment.