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

Consistently extract checking version for all adapters #34232

Merged
merged 1 commit into from Oct 17, 2018

Conversation

Projects
None yet
3 participants
@kamipo
Member

kamipo commented Oct 16, 2018

I don't prefer to extract it for one adapter even though all adapters
also does.

Related to #34227.

Consistently extract checking version for all adapters
I don't prefer to extract it for one adapter even though all adapters
also does.

Related to #34227.

@rails-bot rails-bot bot added the activerecord label Oct 16, 2018

@jeremy

jeremy approved these changes Oct 16, 2018

@kamipo kamipo merged commit bb0c02e into rails:master Oct 17, 2018

2 checks passed

codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@kamipo kamipo deleted the kamipo:check_version branch Oct 17, 2018

koic added a commit to koic/oracle-enhanced that referenced this pull request Oct 17, 2018

Add a check to supported Oracle database version
Follow up of rails/rails#34232.

This PR adds a check to supported Oracle database version.

koic added a commit to koic/oracle-enhanced that referenced this pull request Oct 17, 2018

Add a check to supported Oracle database version
Follow up of rails/rails#34232.

This PR adds a check to supported Oracle database version.

koic added a commit to koic/oracle-enhanced that referenced this pull request Oct 18, 2018

Add a check to supported Oracle database version
Follow up of rails/rails#34232.

This PR adds a check to supported Oracle database version.
@haberbyte

This comment has been minimized.

Contributor

haberbyte commented Oct 18, 2018

Just wanted to note here that before this commit "rgeo/activerecord-postgis-adapter" works with the master branch of Rails (apart from the constrained dependency).

After this commit I get:

NoMethodError (undefined method `server_version' for nil:NilClass):

rails (885ab065b50e) activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb:406:in `postgresql_version'
rails (885ab065b50e) activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb:415:in `check_version'
rails (885ab065b50e) activerecord/lib/active_record/connection_adapters/abstract_adapter.rb:129:in `initialize'
rails (885ab065b50e) activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb:220:in `initialize'
activerecord-postgis-adapter (7cc331d3f599) lib/active_record/connection_adapters/postgis_adapter.rb:60:in `initialize'
activerecord-postgis-adapter (7cc331d3f599) lib/active_record/connection_adapters/postgis/create_connection.rb:38:in `new'
activerecord-postgis-adapter (7cc331d3f599) lib/active_record/connection_adapters/postgis/create_connection.rb:38:in `postgis_connection'
rails (885ab065b50e) activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:811:in `new_connection'
rails (885ab065b50e) activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:855:in `checkout_new_connection'
rails (885ab065b50e) activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:834:in `try_to_checkout_new_connection'
rails (885ab065b50e) activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:795:in `acquire_connection'
rails (885ab065b50e) activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:523:in `checkout'
rails (885ab065b50e) activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:382:in `connection'
rails (885ab065b50e) activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb:1010:in `retrieve_connection'
rails (885ab065b50e) activerecord/lib/active_record/connection_handling.rb:181:in `retrieve_connection'
rails (885ab065b50e) activerecord/lib/active_record/connection_handling.rb:153:in `connection'
rails (885ab065b50e) activerecord/lib/active_record/migration.rb:555:in `call'

I guess the postgis adapter would need to now implement its version of the check_version method, right?

kamipo added a commit to kamipo/activerecord-postgis-adapter that referenced this pull request Oct 19, 2018

Ensure connection before `PostGISAdapter#initialize`
Since rails/rails#34232, early checking server version in superclass so no longer accept nil connection.
@kamipo

This comment has been minimized.

Member

kamipo commented Oct 19, 2018

Thanks for the report. Since this change, PostGISAdapter#initialize no longer accept nil connection.

that error should be fixed by rgeo/activerecord-postgis-adapter#292.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment