Skip to content
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

db:test:purge fails when db is set with DATABASE_URL only #24294

Closed
Kukunin opened this issue Mar 24, 2016 · 12 comments
Closed

db:test:purge fails when db is set with DATABASE_URL only #24294

Kukunin opened this issue Mar 24, 2016 · 12 comments

Comments

@Kukunin
Copy link
Contributor

Kukunin commented Mar 24, 2016

It's identical to this issue #13742, but for Rails 5.

Steps to reproduce

Create application, set database with DATABASE_URL only, but without config/database.yml config file.

Run bundle exec rake db:test:load

Expected behavior

Command works successfully, without any error, with using DB config from DATABASE_URL env variable.

Actual behavior

** Invoke db:test:load (first_time)
** Invoke db:test:purge (first_time)
** Invoke environment (first_time)
** Execute environment
** Invoke db:load_config (first_time)
** Execute db:load_config
** Invoke db:check_protected_environments (first_time)
** Invoke environment 
** Invoke db:load_config 
** Execute db:check_protected_environments
** Execute db:test:purge
rake aborted!
NoMethodError: undefined method `[]' for nil:NilClass
/usr/local/bundle/gems/activerecord-5.0.0.beta3/lib/active_record/tasks/database_tasks.rb:185:in `purge'
/usr/local/bundle/gems/activerecord-5.0.0.beta3/lib/active_record/railties/databases.rake:369:in `block (3 levels) in <top (required)>'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:248:in `block in execute'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:243:in `each'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:243:in `execute'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:187:in `block in invoke_with_call_chain'
/usr/local/lib/ruby/2.3.0/monitor.rb:214:in `mon_synchronize'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:180:in `invoke_with_call_chain'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:209:in `block in invoke_prerequisites'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:207:in `each'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:207:in `invoke_prerequisites'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:186:in `block in invoke_with_call_chain'
/usr/local/lib/ruby/2.3.0/monitor.rb:214:in `mon_synchronize'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:180:in `invoke_with_call_chain'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:173:in `invoke'
/app/lib/tasks/ci.rake:38:in `block (2 levels) in <top (required)>'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:248:in `block in execute'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:243:in `each'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:243:in `execute'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:187:in `block in invoke_with_call_chain'
/usr/local/lib/ruby/2.3.0/monitor.rb:214:in `mon_synchronize'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:180:in `invoke_with_call_chain'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:209:in `block in invoke_prerequisites'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:207:in `each'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:207:in `invoke_prerequisites'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:186:in `block in invoke_with_call_chain'
/usr/local/lib/ruby/2.3.0/monitor.rb:214:in `mon_synchronize'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:180:in `invoke_with_call_chain'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/task.rb:173:in `invoke'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/application.rb:150:in `invoke_task'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/application.rb:106:in `block (2 levels) in top_level'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/application.rb:106:in `each'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/application.rb:106:in `block in top_level'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/application.rb:115:in `run_with_threads'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/application.rb:100:in `top_level'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/application.rb:78:in `block in run'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/application.rb:176:in `standard_exception_handling'
/usr/local/bundle/gems/rake-11.1.1/lib/rake/application.rb:75:in `run'
/usr/local/bundle/gems/rake-11.1.1/bin/rake:33:in `<top (required)>'
/usr/local/bundle/bin/rake:16:in `load'
/usr/local/bundle/bin/rake:16:in `<main>'

System configuration

Rails version:
5.0.0.beta3

Ruby version:
2.3.0

@Kukunin
Copy link
Contributor Author

Kukunin commented Mar 24, 2016

Reproducible for latest (878c2bb) master

@prathamesh-sonpatki
Copy link
Member

I will work on it.

@sgrif
Copy link
Contributor

sgrif commented Mar 30, 2016

This issue appears to occur on 4.2 as well. Since this issue is not a regression in 5.0, it can be resolved in 5.0.1 but does not need to block the initial release.

@sgrif sgrif removed this from the 5.0.0 milestone Mar 30, 2016
@novotarq
Copy link

novotarq commented Apr 8, 2016

The problem is, that ActiveRecord::Base.configurations does only contain data for development when using DATABASE_URL , here is the parsed config:
{"development"=>{"adapter"=>"sqlite3", "database"=>"//vagrant/rails/my-test-app/db.sqlite"}}

From what I have seen, there are at least two solutions:
The first one would be to add configuration for the test database here:
https://github.com/rails/rails/blob/master/activerecord/lib/active_record/connection_handling.rb#L75

Second approach would be to just change the config in Rake task:
https://github.com/rails/rails/blob/master/activerecord/lib/active_record/railties/databases.rake#L369
ActiveRecord::Tasks::DatabaseTasks.purge ActiveRecord::Base.configurations['test'] || ActiveRecord::Base.configurations['development']

I think the latter would be much simpler. Any opinions?

@vipulnsward
Copy link
Member

Hi @prathamesh-sonpatki still working on this? Was looking on related code and fix for this.
If not, I can take a stab at this.

@prathamesh-sonpatki
Copy link
Member

Yes, I am working on this.

@mayurkhatri
Copy link

@Kukunin could you please help me with reproducing this?

@arthurnn arthurnn self-assigned this Oct 27, 2016
@arthurnn
Copy link
Member

@prathamesh-sonpatki ping me on the PR resolving this issue, I would like to take a look.

@prathamesh-sonpatki
Copy link
Member

prathamesh-sonpatki commented Oct 29, 2016

@Kukunin @arthurnn This is a tricky issue.

The database test rake tasks don't rely on the ENV for manipulating test database. They directly read from the "test" configuration specification.

Additionally, DATABASE_URL gets applied to the current ENV when it gets resolved.

When we are using the test rake tasks within a Rails app, the environment used by the connection handling logic is "development" unless specified by ENV['RAILS_ENV'] or ENV['RACK_ENV'].

So the DATABASE_URL gets resolved to only the "development" environment and fetching it for "test" environment fails.

IMO resolving DATABASE_URL to test environment is bad because it means that your database gets manipulated when running the tests. This defeats the purpose of having the "Rails" way of separate databases for development, test and production.

In this, we can update the documentation and raise a friendly error saying that You have not configured the "test" database. Please do that using config/database.yml.

Let me know your thoughts and I can work on a PR for this approach.

@Kukunin
Copy link
Contributor Author

Kukunin commented Nov 1, 2016

@prathamesh-sonpatki I see what's the problem.

Actually I agree with you.

If one is going to run Rails application in test env with database set via DATABASE_URL, explicit RAILS_ENV=test should work.

Otherwise, replace db:test:purge with command that manipulates with current database

@prathamesh-sonpatki
Copy link
Member

Otherwise, replace db:test:purge with command that manipulates with current database

This not a good option. db:test:purge means purge test database.

We can document this case with the DATABASE_URL and also raise helpful error message for missing test database configuration.

prathamesh-sonpatki added a commit to prathamesh-sonpatki/rails that referenced this issue Jan 2, 2017
…tabase is not configured

- Inspiration for this change comes from rails#24294
  where `db:test:purge` fails when used with DATABASE_URL.
- This is because the database rake tasks don't depend on RAILS_ENV
  and that's why DATABASE_URL gets applied to development environment
  by default when running these tasks.
- This means that the "test" database configuration is not present but
  while running test database rake tasks we expect it to be present.
- This commit will raise a helpful error when "test" database
  configuration is not present, gently reminding users that they need
  to add it before running the test database rake tasks.
@stale
Copy link

stale bot commented Mar 27, 2017

This issue has been automatically marked as stale because it has not been commented on for at least three months.
The resources of the Rails team are limited, and so we are asking for your help.
If you can still reproduce this error on the 5-1-stable branch or on master, please reply with all of the information you have about it in order to keep the issue open.
Thank you for all your contributions.

@stale stale bot added the wontfix label Mar 27, 2017
@stale stale bot closed this as completed Apr 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants