-
Notifications
You must be signed in to change notification settings - Fork 11
Extend configuration to support disable_during_db_migrate
#34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 20 commits
704b232
da2eae7
f3a7626
7b747ab
8a78a4e
d261ac9
037920a
9a60340
220bd6b
7ab0d57
f8214ae
0eedded
b796e6d
4a99cfe
6a710aa
1e33d77
c18f7a8
c44c1b8
f7ad848
29dab50
ea45172
b1b6c08
1bd0b61
ce6fc81
fcdedf2
9b37fda
fbc41b4
12bcb2a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,10 +6,17 @@ module PpSql | |
| # if you do not want to rewrite AR native method #to_sql | ||
| # you may switch this setting to false in initializer | ||
| class << self | ||
| attr_accessor :rewrite_to_sql_method, :add_rails_logger_formatting | ||
| attr_accessor :rewrite_to_sql_method, :add_rails_logger_formatting, :disable_for_db_tasks | ||
|
|
||
| def enabled_for?(action) | ||
| return false if ENV['PPSQL_DISABLED'] | ||
|
|
||
| send(action) | ||
| end | ||
| end | ||
| self.rewrite_to_sql_method = true | ||
| self.add_rails_logger_formatting = true | ||
| self.disable_for_db_tasks = true | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From testing, something I didn't quite realize is that this file and the rake tasks are loaded before any of the local project files/initializers are read. This means that setting |
||
|
|
||
| module Formatter | ||
| private | ||
|
|
@@ -28,7 +35,7 @@ def _sql_formatter | |
|
|
||
| module ToSqlBeautify | ||
| def to_sql | ||
| if ::PpSql.rewrite_to_sql_method | ||
| if ::PpSql.enabled_for?(:rewrite_to_sql_method) | ||
| extend Formatter | ||
| _sql_formatter.format(defined?(super) ? super.dup : dup) | ||
| else | ||
|
|
@@ -37,7 +44,7 @@ def to_sql | |
| end | ||
|
|
||
| def pp_sql | ||
| if ::PpSql.rewrite_to_sql_method | ||
| if ::PpSql.enabled_for?(:rewrite_to_sql_method) | ||
| puts to_sql | ||
| else | ||
| extend Formatter | ||
|
|
@@ -50,7 +57,7 @@ module LogSubscriberPrettyPrint | |
| include Formatter | ||
|
|
||
| def sql(event) | ||
| return super unless ::PpSql.add_rails_logger_formatting | ||
| return super unless ::PpSql.enabled_for?(:add_rails_logger_formatting) | ||
|
|
||
| e = event.dup | ||
| e.payload[:sql] = _sql_formatter.format(e.payload[:sql].dup) | ||
|
|
@@ -66,6 +73,11 @@ class Railtie < Rails::Railtie | |
| ActiveRecord::LogSubscriber.prepend LogSubscriberPrettyPrint | ||
| end | ||
| end | ||
|
|
||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rake_tasks do
path = File.expand_path(__dir__)
load("#{path}/pp_sql/tasks/db_hooks.rake") unless PpSql.enable_for_rails_rake_db_tasks
end |
||
| rake_tasks do | ||
| path = File.expand_path(__dir__) | ||
| Dir.glob("#{path}/pp_sql/tasks/**/*.rake").each { |f| load f } | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| namespace :pp_sql do | ||
| desc 'Hook that is ran before rails `db:*` tasks that can disable pp_sql' | ||
|
|
||
| task :disable_during_db_tasks do | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. task :disable_during_db_tasks do
PpSql.add_rails_logger_formatting = false
PpSql.rewrite_to_sql_method = false
end
end |
||
| if PpSql.disable_for_db_tasks | ||
| PpSql.add_rails_logger_formatting = false | ||
| PpSql.rewrite_to_sql_method = false | ||
| end | ||
| end | ||
| end | ||
|
|
||
| db_tasks = Rake::Task.tasks.select { |task| task.name.starts_with?('db:') } | ||
| db_tasks.each { |task| task.enhance(['pp_sql:disable_during_db_tasks']) } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's a bit odd, since with this lately u'r wrapping the boolean settings with another one. Seems to be excessive
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I was originally using a
disabled_accessor the logic was feeling hard to read (if enabled && !ENV[disabled]) so pulling it into a single method felt cleaner. However, based on your other suggestions the disabled ENV can go away so I'll remove this method.