Rails 6.1.0.rc1 unable to establish connection to the database that holds our sessions

[codeodor]

In Rails 6.0, we were able to connect the ActiveRecord Session Store to the database that holds our sessions, like this:

ActiveRecord::SessionStore::Session.connects_to database: { reading: "session_database_#{Rails.env}".to_sym, writing: "session_database_#{Rails.env}".to_sym }

However, in Rails 6.1.0.rc1, this gives an error:

NotImplementedError: `connects_to` can only be called on ActiveRecord::Base or abstract classes

As we cannot change the base class of ActiveRecord::SessionStore::Session, will there be (or is there) some other way to tell Rails that the session should always be fetched from a specific database?

[codeodor]

Would you accept a PR that adds a SessionBase abstract class that inherits from ActiveRecord::Base, and then switch to inheriting from it, so we can have some control over which database it will use by doing something like this?

module ActiveRecord
  module SessionStore
    # The default Active Record class.
    class SessionBase < ActiveRecord::Base
      self.abstract_class = true
    end 

    class Session < SessionBase
      # .... existing code here
    end
  end
end

Then in our code, we could do this?

ActiveRecord::SessionStore::SessionBase.connects_to database: { reading: "session_database_#{Rails.env}".to_sym, writing: "session_database_#{Rails.env}".to_sym }

[codeodor]

It was easy enough to try, and seems to fix my problem, so I went ahead with the PR. I says seems because I have a lot of errors in my upgrade to get through, but I hope that will solve this one.

[codeodor]

The solution we ended up with was having our session_store.rb initializer look like this:

Rails.application.config.session_store :active_record_store

class AppSessionBase < ActiveRecord::SessionStore::Session
  self.abstract_class = true

  connects_to database: { reading: :session_primary, writing: :session_primary }
end

class AppSession < AppSessionBase
  # needed b/c AppSessionBase is "abstract and cannot be instantiated"
end

ActionDispatch::Session::ActiveRecordStore.session_class = AppSession

[sikachu]

As I mentioned in #169, I think the fact that you have to go around and make another abstract class to achieve this functionality doesn't sound right to me, so let's find an alternative solution.

Let me check with people who familiar with the multi-database stuff to see what they would recommend.

Generally, I think if you, say, set the connects_to on ActiveRecord::Base, it should already being picked up by ActiveRecord::SessionStore::Session, right?

[sikachu]

Ok, I've discussed with people who's familiar with the issue, and I think I have a conclusion on how we should support this.

Note that based on @eileencodes's comment in rails/rails#40219 (comment), I don't think it's wise to suggest user to use connects_to on a subclass of AR::Base unless it's really necessary since it can cause your MySQL server to go out of connection.

However, if user understands the caveat, then I think will happily accepting a PR that makes AR::SS::Session inherits from an abstract class similar to what you suggested in #167 (comment). We'd also need a documentation in README.md suggesting user on how to configure it via AbstractSession class.

[sikachu]

I just noticed #168 ... sorry for the confusion 🤦

Still, I'm tempted that we should add support for that in our gem instead of asking user to create a new abstract class in their app as I said above, so maybe I'll reopen #168 and merge it.

@eileencodes can I have your second opinion on this?

[eileencodes]

If you have multiple databases creating a new abstract class isn't going to be that many more steps than if we made an abstract class. App users would still need to add connects_to which can't be done on non-abstract classes.

So I don't want to merge #168, it only needs to be abstract for multi-db and in those cases since you need to explicitly connect to the right database, making the abstract class in your app is the right way to fix the problem described here.

[h0jeZvgoxFepBQ2C]

Is there any update about this?

[codeodor]

@h0jeZvgoxFepBQ2C we went with this solution:

#167 (comment)