skip if `#has_flags` is called by running `rails db:create`

ebihara99999 commented 6 years ago

What happened?

rails db:create fails with the following trace:

rails aborted!
ActiveRecord::NoDatabaseError: Unknown database 'myproject_development'
/app/myproject/vendor/bundle/ruby/2.5.0/gems/activerecord-5.2.0/lib/active_record/connection_adapters/mysql2_adapter.rb:26:in `rescue in mysql2_connection'
/app/myproject/vendor/bundle/ruby/2.5.0/gems/activerecord-5.2.0/lib/active_record/connection_adapters/mysql2_adapter.rb:12:in `mysql2_connection'
/app/myproject/vendor/bundle/ruby/2.5.0/gems/activerecord-5.2.0/lib/active_record/connection_adapters/abstract/connection_pool.rb:809:in `new_connection'
/app/myproject/vendor/bundle/ruby/2.5.0/gems/activerecord-5.2.0/lib/active_record/connection_adapters/abstract/connection_pool.rb:853:in `checkout_new_connection'
/app/myproject/vendor/bundle/ruby/2.5.0/gems/activerecord-5.2.0/lib/active_record/connection_adapters/abstract/connection_pool.rb:832:in `try_to_checkout_new_connection'
/app/myproject/vendor/bundle/ruby/2.5.0/gems/activerecord-5.2.0/lib/active_record/connection_adapters/abstract/connection_pool.rb:793:in `acquire_connection'
/app/myproject/vendor/bundle/ruby/2.5.0/gems/activerecord-5.2.0/lib/active_record/connection_adapters/abstract/connection_pool.rb:521:in `checkout'
/app/myproject/vendor/bundle/ruby/2.5.0/gems/activerecord-5.2.0/lib/active_record/connection_adapters/abstract/connection_pool.rb:380:in `connection'
/app/myproject/vendor/bundle/ruby/2.5.0/gems/activerecord-5.2.0/lib/active_record/connection_adapters/abstract/connection_pool.rb:1008:in `retrieve_connection'
/app/myproject/vendor/bundle/ruby/2.5.0/gems/activerecord-5.2.0/lib/active_record/connection_handling.rb:118:in `retrieve_connection'
/app/myproject/vendor/bundle/ruby/2.5.0/gems/activerecord-5.2.0/lib/active_record/connection_handling.rb:90:in `connection'
/app/myproject/vendor/bundle/ruby/2.5.0/gems/switch_point-0.8.0/lib/switch_point/model.rb:93:in `connection'
/app/myproject/vendor/bundle/ruby/2.5.0/bundler/gems/flag_shih_tzu-b07b5a721112/lib/flag_shih_tzu.rb:365:in `check_flag_column'
/app/myproject/vendor/bundle/ruby/2.5.0/bundler/gems/flag_shih_tzu-b07b5a721112/lib/flag_shih_tzu.rb:39:in `has_flags'
/app/myproject/app/models/product.rb:92:in `<class:Product>'

#has_flags is called in /app/myproject/app/models/product.rb:92.

Envirinment

Ruby: 2.5.0
Rails: 5.2.0

How to fix

Catch ActiveRecord::NoDatabaseError and do nothing. It fails when run rails db:create because #connection raises ActiveRecord::NoDatabaseError. It's not necessary to check what has_flags does if it's in database creation.

coveralls commented 6 years ago

Coverage decreased (-54.3%) to 0.0% when pulling 8c248dbf6a578056580f02e8a8aa54c95489f392 on ebihara99999:fix-error-raised-in-db-creation into b07b5a72111240cd5cb56a96722e80debf691fd0 on pboling:master.

coveralls commented 6 years ago

Coverage increased (+0.1%) to 54.444% when pulling 4e60d44cfbd2ec20cab817bd39f69a28d9ed7194 on ebihara99999:fix-error-raised-in-db-creation into b07b5a72111240cd5cb56a96722e80debf691fd0 on pboling:master.

pboling commented 6 years ago

This fix is constrained to those versions of ActiveRecord that have that error class, so I can't merge this without some additional checks there. Also, there is a built-in option to skip the database check already which should avoid this error.

ebihara99999 commented 6 years ago

Thank you for the comment!

Do you mean the built-in option by Skipping flag column check referred to in README.md?

The feature seems to require users to set the flag false at first, and then to set the flag true once database creation is finished. It would be somehow inconvenient. I personally think this PR is better in the view of avoiding the error in database creation.

If you would review again when I reflect your correction, check more errors than ActiveRecord::NoDatabaseError, I will check other errors and add them if there is.

ebihara99999 commented 6 years ago

@pboling NoDatabaseError is introduced in 4.1 stable branch. Before that, there seems no codes handling kind of error.

Do you think there is necessity to check more?

pboling commented 6 years ago

@ebihara99999 Sorry if I wasn't clear enough. The current gem supports Rails down to 2.3. I would like to shed all version of Rails prior to 4.2 in a 1.0 release, but I haven't had time to work on that.

Because this gem supports 2.3 we can't introduce a change that would break on any version prior to 4.1. The gem needs to continue to work on older rails until that support is officially dropped.

If we reference an error class in the code that doesn't exist in earlier versions of rails, then projects on those old version will raise an error about an undefined constant, or a NameError. We would need to have a guard that checks for the existence of the constant before trying to use the constant.

ebihara99999 commented 6 years ago

@pboling Thanks, I got the point. Would you review again?

pboling commented 6 years ago

@ebihara99999 The latest change is great. Any idea about the failing build?

ebihara99999 commented 6 years ago

@pboling I'll look into it when I have time.

pboling commented 6 years ago

@ebihara99999 I have fixed the failing build issues on master. Thanks!

pboling / flag_shih_tzu