-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
When I run ENV=development rake db:create I don't expect it to try and create the test db too #27299
Comments
Here is some example output:
An example of where this is a bit odd, is when test is an sqlite3 in-memory database. Therefore, get this output every time. |
So, here is the odd behyavior from db:drop
I don't think that's good behaviour. |
In the same vein, I'd like to point out the following inconsistencies: task :environment do
# This must be a symbol... or establish_connection thinks it's a URL.
DATABASE_ENV = :development
ActiveRecord::Base.configurations = {
# This key must be a string or it will not be matched by ActiveRecord:
'development' => {
'adapter' => 'sqlite3',
# This key must be a string or rake tasks will fail (e.g. each_current_configuration fails):
'database' => 'db/development.db'
}
}
# Connect to database:
unless ActiveRecord::Base.connected?
ActiveRecord::Base.establish_connection(DATABASE_ENV)
end
end I'd be happy to take a look at these issues, but I'd need to know if the Rails core team consider these to be problems or not. |
What's even more.. inconsistent.. is the fact that once you actually connect, all the keys are symbols: > Account.connection_config
=> {:adapter=>"mysql2", :database=>"vmail_development", :mail_root=>"/tmp/mail_root"} |
Looks like a violation of the least surprise principle to me as well. Experienced that issue many times |
Honestly, the db:* tasks seem like a bit of a mess. For all the above reasons.. and the way they tie into Rails by default, which breaks unless you override a magical set of variables on "module DatabaseTasks" which in the documentation is described as a class. |
What kind of feedback would you like? |
I've been hacking up a solution. Something like this: def each_current_configuration(environment)
unless configuration = ActiveRecord::Base.configurations[environment]
raise ArgumentError.new("Cannot find configuration for environment #{environment}")
end
if configuration['database'].blank?
raise ArgumentError.new("Configuration for environment #{environment} doesn't specify database")
end
yield configuration
end |
So, I've been working on a gem which fixes all these issues (and more). https://github.com/ioquatix/activerecord-migrations I'm not really sure if this is a solution, it's more of a monkey patch. But it solves a lot of pain points for us w.r.t. deployment with active record, which to be honest leaves a lot to be desired. |
Just to clarify another inconsistency: ActiveRecord::Base.configurations = {
'production' => Database::APP_PRODUCTION,
'development' => Database::APP_DEVELOPMENT,
'test' => Database::TEST,
}
DATABASE_ENV = :production
# Access key.. via string:
ActiveRecord::Base.configurations[DATABASE_ENV.to_s]
# Actually connect.. need to use symbol:
ActiveRecord::Base.establish_connection[DATABASE_ENV] If you try to use symbols in configurations, it breaks in other ways. If you try to use a string in |
Just chiming in here... We have been troubleshooting this issue today as well. It seems to just disregard our RAILS_ENV and tries to create the test db as well as our development db. This causes issues in our case since we have completely different setups for development and test, so it errors and says it can't create the test db. |
@JCSHoosier take a look at https://github.com/ioquatix/activerecord-migrations it fixes those issues by monkey patch. |
@ioquatix Yep, looking through it now! Thanks! |
This issue has been automatically marked as stale because it has not been commented on for at least three months. |
This issue still can be occurred in rails 5.1.0.rc1 The But it is more concerning that dropping the test database drops the development one as well. Rails version: 5.1.0.rc1 |
Can someone from the Rails core team explain the rationale behind adding rails/activerecord/lib/active_record/tasks/database_tasks.rb Lines 286 to 288 in 3f27997
|
Yes. The reason is |
See also 6ca9031 |
Why wouldn't rails test just create the test database if required? Wouldn't that make more sense? |
At first look yes, but that would also require to other tests frameworks to be changed and in this case I prefer to play safer and keep the behavior that is being like this for years. |
How can we make this not break our staging/development environment which most certainly doesn't want a test database. |
Maybe implementing the alternative suggested https://github.com/rails/rails/pull/24201/files#r56104919 and applications overriding the task to do what they want. |
well, I guess I'm not the only one facing this 'issue'. |
It's pretty strange to me there is no way to override this behavior. I understand the Rails Core team expects this behavior, and wants new users to hit the ground running with simple commands, etc. That's fine. But we're using AWS' ParameterStore to manage our credentials, so every time we make an unnecessary, and unexpected query, it costs more money and creates more issues for us. Even if we accept that this behavior is "expected," despite it being surprising, I still believe there should be a way to override this behavior with a switch or something. |
If we have different database connections for test vs development, it appears this new scheme doesn't work at all. Whatever login we use for test (using a ramdisk for speed) doesn't work for development (using persistent disk for large amounts of test data). And there's no easy workaround that I can see, apart from patching the source. |
this should be configurable. |
I got bitten by this today as well... Wasted about 4 hours looking into this particular issue until I found this thread. If you add up all the hours everyone has invested in this not so obvious behavior you might be impressed by the amount of money that is being wasted because of an ego. Quick advise: Listen to your users. |
@rafaelfranca Do you think there is any way we can revisit the decision here? |
No. This is by design. |
But, I'm open to an environment variable to disable that if that is what people want. |
I think that would definitely help. When I ran into this years ago it was such a pain due to how we had everything configured. @tinco 's suggestion above would help. This also should be documented as an expected behavior somewhere (with the flag to disable). |
I would find this useful. At the moment, we are copying around dummy database config files in our CI/CD pipelines. |
Maybe when How about that? |
I came across this yesterday too. @rafaelfranca I tried the environment variable approach you described: #39027 |
@JCSHoosier Actually this behaviour is well documented.
Anyway creating only specified database should be somehow supported. Adding ENV variables is not systematic solution. If we should to make this configurable, I think it should be added as config option for active record and you can tweak this config using ENV variable on your own the same we do for example for static files: rails/railties/lib/rails/generators/rails/app/templates/config/environments/production.rb.tt Line 25 in 7ee8d7e
|
🤔 That will make environment having no |
I don't want this to be configurable. I'm not against people using this, but I also don't want this becoming mainstream feature. There are clearly some use cases like creating test database in paid machine where creating those databases are expensive but this is not something everyone needs. For those applications an environment variable is enough. |
Would it be possible to get the best of both worlds? Keep It might require some internal refactoring, but from an end user point of view, it should be possible, right? |
This will be available in the next rails release? |
I was working on a rails 4 legacy app, and I got assigned to another project using rails 5. When I did my usual |
If you landed here like me #39027 has seem to fixed this with SKIP_TEST_DATABASE=true or ENV["SKIP_TEST_DATABASE"]=true |
What if it's the opposite and you want to drop/create a test db without creating a development db? Is there a solution for that? #39027 doesn't solve that issue. |
This issue is about the implication that |
Steps to reproduce
According to the following code:
rails/activerecord/lib/active_record/tasks/database_tasks.rb
Lines 286 to 288 in 3f27997
When running something like this:
It also tries to create test.
I think this is violating the principle of least surprise. At a cursory glance, this also applies to
drop_current
which might be even more surprising.Expected behavior
Only development environment is created.
Actual behavior
It tries to create test database too.
System configuration
Latest.
The text was updated successfully, but these errors were encountered: