-
Notifications
You must be signed in to change notification settings - Fork 24
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
Shender/rails 4.2 test env db creation fix #38
Conversation
@@ -4,16 +4,22 @@ | |||
Rake::Task[name].clear | |||
end | |||
|
|||
def env_name |
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.
let's not define overly generic global methods -> static class methods
module ActiveRecordShards
module Tasks
def self.env_name
....
end
end
end
logic looks good to me, a little confused on why nobody ran into this before ... it's what vanilla rails does too so 👍 |
ActiveRecord::Base.configurations.each do |key, conf| | ||
if key.starts_with?(env_name) && !key.ends_with?("_slave") | ||
begin | ||
if ActiveRecord::VERSION::MAJOR >= 4 | ||
connection = ActiveRecord::Base.send("#{conf['adapter']}_connection", conf.merge('database' => nil)) | ||
connection.drop_database(conf['database']) | ||
root_connection(conf).drop_database(conf['database']) |
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.
ah, nice approach. I've banged my head on the right way to do this without completely side-stepping active record, this is pretty decent. 👍 after grosser's review fixes.
Use .include? instead of .match. Change output from stderr to stdout.
@grosser |
module ActiveRecordShards | ||
module Tasks | ||
def self.env_name | ||
ActiveRecordShards.rails_env || 'development' |
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.
maybe ActiveRecordShards.rails_env
could already include the || 'development'
👍 |
Made the final change @grosser , and will merge in when Travis comes back with the go-ahead. Thanks! |
@@ -51,6 +51,7 @@ def self.rails_env | |||
env = Rails.env if defined?(Rails.env) | |||
env ||= RAILS_ENV if Object.const_defined?(:RAILS_ENV) | |||
env ||= ENV['RAILS_ENV'] | |||
env ||= 'development' |
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.
good change since we also call .dup
on it :D
👍 |
…ation_fix Shender/rails 4.2 test env db creation fix
merged! |
I was having problems with creating DBs in the test environment. Now the 'drop' and 'create' are consistent.
/cc @gabetax @osheroff @staugaard @grosser @bquorning
References
Risks