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

Switch to using an abstract base class for Session #168

Closed
wants to merge 1 commit into from

Conversation

codeodor
Copy link
Contributor

This allows other programmers to specify the correct database connection criteria
See #167

This allows other programmers to specify the correct database connection criteria
See rails#167
@rafaelfranca
Copy link
Member

@eileencodes does this change makes sense?

@eileencodes
Copy link
Member

Will it work to do this in your app instead?

class AppSession < ActiveRecord::SessionStore::SessionBase
  self.abstract_class = true

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

Also, unrelated, it looks like you're not using a 3-tier config. I don't recommend using multiple databases without the 3-tier. I guess it can work but we make no guarantee.

Your config should look like this:

development:
  session_store_primary:
    database: db_name
  session_store_replica:
    database: db_name
    replica: true

Then your app would call connects_to like this:

connects_to database: { writing: :session_store_primary, reading: :session_store_replica }

This way Rails can just find the right config rather than doing the to_sym dance.

@codeodor
Copy link
Contributor Author

codeodor commented Dec 1, 2020

That seems like a good idea. Then, @rafaelfranca I guess all I'd need to do is:

ActionDispatch::Session::ActiveRecordStore.session_class = AppSession

in our session_store.rb initializer and everything would just hook up correctly?

I'll give it a try and let you all know.

@codeodor
Copy link
Contributor Author

codeodor commented Dec 2, 2020

That seems to work!

Just a note for anyone who finds this later via google, the code I used in our session_store.rb initializer was

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

Also, thanks @eileencodes for the tip on the 3 tiers. We are using them, but when we moved to Rails 6 I originally had it set up in a way that included the environment because we were naming our keys after the database itself. I think I was trying to make it easy for me to move back and forth between styles as we migrated, but no need to keep it that way now.

@codeodor codeodor closed this Dec 2, 2020
@eileencodes
Copy link
Member

Glad it worked! 😄 Do you want to open a PR to add this to the documentation?

@codeodor
Copy link
Contributor Author

codeodor commented Dec 2, 2020

Good idea, I just did that. Thanks!

@base10
Copy link

base10 commented Mar 17, 2023

Still helpful. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants