39 legacy connection handler #40
Conversation
lib/grape/app/initializers/pre.rb
Outdated
| @@ -8,7 +8,7 @@ | |||
| ActiveSupport::KeyGenerator.hash_digest_class = OpenSSL::Digest::SHA256 if ActiveSupport::KeyGenerator.respond_to?(:hash_digest_class=) | |||
| ActiveSupport::MessageEncryptor.use_authenticated_message_encryption = true | |||
| ActiveSupport::IsolatedExecutionState.isolation_level = :thread if defined?(ActiveSupport::IsolatedExecutionState) | |||
| Digest::UUID.use_rfc4122_namespaced_uuids = true if Digest::UUID.respond_to?(:use_rfc4122_namespaced_uuids=) | |||
| # Digest::UUID.use_rfc4122_namespaced_uuids = true if Digest::UUID.respond_to?(:use_rfc4122_namespaced_uuids=) | |||
There was a problem hiding this comment.
I'm not sure about this one. Checking....
There was a problem hiding this comment.
It's fine, activesupport is using SecureRandom.uuid nowadays, this is legacy from when it used Digest::UUID directly. This changed in 7.1, but OK to keep as we support older activesupport versions too
There was a problem hiding this comment.
just add a comment saying, this is legacy, removed with Rails 7.1 so we know in the future
lib/grape/app/initializers/pre.rb
Outdated
| @@ -8,7 +8,7 @@ | |||
| ActiveSupport::KeyGenerator.hash_digest_class = OpenSSL::Digest::SHA256 if ActiveSupport::KeyGenerator.respond_to?(:hash_digest_class=) | |||
| ActiveSupport::MessageEncryptor.use_authenticated_message_encryption = true | |||
| ActiveSupport::IsolatedExecutionState.isolation_level = :thread if defined?(ActiveSupport::IsolatedExecutionState) | |||
| Digest::UUID.use_rfc4122_namespaced_uuids = true if Digest::UUID.respond_to?(:use_rfc4122_namespaced_uuids=) | |||
| # Digest::UUID.use_rfc4122_namespaced_uuids = true if Digest::UUID.respond_to?(:use_rfc4122_namespaced_uuids=) | |||
There was a problem hiding this comment.
It's fine, activesupport is using SecureRandom.uuid nowadays, this is legacy from when it used Digest::UUID directly. This changed in 7.1, but OK to keep as we support older activesupport versions too
| @@ -43,12 +43,6 @@ def call(exception, *) | |||
| ActiveRecord::Base.default_timezone = :utc | |||
| end | |||
|
|
|||
| if ActiveRecord.respond_to?(:legacy_connection_handling=) | |||
| ActiveRecord.legacy_connection_handling = false | |||
| else | |||
There was a problem hiding this comment.
should this be an elsif. same here, we should support versions older than 7.1
spec/grape/app_spec.rb
Outdated
| @@ -43,14 +43,13 @@ def app | |||
| if ActiveSupport::VERSION::MAJOR >= 7 | |||
| expect(ActiveSupport::KeyGenerator.hash_digest_class).to be(OpenSSL::Digest::SHA256) | |||
| expect(ActiveSupport::IsolatedExecutionState.isolation_level).to be(:thread) | |||
| expect(Digest::UUID.use_rfc4122_namespaced_uuids).to be(true) | |||
| # expect(Digest::UUID.use_rfc4122_namespaced_uuids).to be(true) | |||
lib/grape/app/initializers/pre.rb
Outdated
| @@ -8,7 +8,7 @@ | |||
| ActiveSupport::KeyGenerator.hash_digest_class = OpenSSL::Digest::SHA256 if ActiveSupport::KeyGenerator.respond_to?(:hash_digest_class=) | |||
| ActiveSupport::MessageEncryptor.use_authenticated_message_encryption = true | |||
| ActiveSupport::IsolatedExecutionState.isolation_level = :thread if defined?(ActiveSupport::IsolatedExecutionState) | |||
| Digest::UUID.use_rfc4122_namespaced_uuids = true if Digest::UUID.respond_to?(:use_rfc4122_namespaced_uuids=) | |||
| # Digest::UUID.use_rfc4122_namespaced_uuids = true if Digest::UUID.respond_to?(:use_rfc4122_namespaced_uuids=) | |||
There was a problem hiding this comment.
just add a comment saying, this is legacy, removed with Rails 7.1 so we know in the future
| @@ -43,6 +44,15 @@ def call(exception, *) | |||
| ActiveRecord::Base.default_timezone = :utc | |||
| end | |||
|
|
|||
| # legacy_connection_handling was deprecated in ActiveRecord 7 | |||
| if ActiveRecord::VERSION::MAJOR < 7 | |||
There was a problem hiding this comment.
why not just elsif ActiveRecord::Base.respond_to?(:legacy_connection_handling=). seems more consistent
There was a problem hiding this comment.
So it's still in the code.
https://github.com/rails/rails/blob/v7.1.3/activerecord/lib/active_record.rb#L246
So respond_to still returns true, then it raises an error. I thought best not to try at all if the version was too high.
No description provided.