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

Add multi-database support to Statesman. #522

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

benk-gc
Copy link
Contributor

@benk-gc benk-gc commented Nov 23, 2023

Statesman currently relies heavily on ActiveRecord::Base, either explicitly or implicitly, when querying database connections. This doesn't play well when we have models or transitions which live on a different database, since it forces us to open a query to the primary connection pool.

This is a series of changes to allow Statesman to use the context it has available for the model or transition class, and make use of the appropriate connection.

@benk-gc benk-gc changed the title First pass of multi-db support. DRAFT: Add multi-database support to Statesman Nov 23, 2023
@benk-gc benk-gc force-pushed the benk--multiple-database-support branch 3 times, most recently from 6973746 to ec92542 Compare November 28, 2023 12:05
@benk-gc benk-gc changed the title DRAFT: Add multi-database support to Statesman Add multi-database support to Statesman Nov 28, 2023
@benk-gc benk-gc marked this pull request as ready for review November 28, 2023 12:07
@benk-gc benk-gc changed the title Add multi-database support to Statesman Add multi-database support to Statesman. Nov 28, 2023
@benk-gc benk-gc force-pushed the benk--multiple-database-support branch from ec92542 to e6a1c89 Compare November 30, 2023 11:47
Statesman currently relies heavily on ActiveRecord::Base, either explicitly
or implicitly, when querying database connections. This doesn't play well
when we have models or transitions which live on a different database, since
it forces us to open a query to the primary connection pool.

This is a series of changes to allow Statesman to use the context it has
available for the model or transition class, and make use of the appropriate
connection.

Co-authored-by: Amey Kusurkar <amey1000@gmail.com>
@benk-gc benk-gc force-pushed the benk--multiple-database-support branch from e6a1c89 to 404095a Compare November 30, 2023 12:00
@benk-gc benk-gc merged commit 6e499a8 into master Nov 30, 2023
78 checks passed
@benk-gc benk-gc deleted the benk--multiple-database-support branch November 30, 2023 12:03
This was referenced Nov 30, 2023
jurre added a commit to jurre/statesman that referenced this pull request Dec 21, 2023
In gocardless#522 the ability to
enable gaplock protection manually was removed, likely assuming there
was no need for this since it's enabled by default when using an adapter
that is named `mysql*`, however we use
https://github.com/trilogy-libraries/trilogy which has been gaining some
traction in the community and this change now leaves us without a way to
enable the functionality.

I think it makes most sense to enable the functionality by default when
using this new adapter as well, and keep config requirements minimal, so
I added a little check for that adapter name as well.
dylan-hoefsloot pushed a commit to dylan-hoefsloot/statesman that referenced this pull request Jan 11, 2024
Statesman currently relies heavily on ActiveRecord::Base, either explicitly
or implicitly, when querying database connections. This doesn't play well
when we have models or transitions which live on a different database, since
it forces us to open a query to the primary connection pool.

This is a series of changes to allow Statesman to use the context it has
available for the model or transition class, and make use of the appropriate
connection.

Co-authored-by: Amey Kusurkar <amey1000@gmail.com>
dylan-hoefsloot pushed a commit to dylan-hoefsloot/statesman that referenced this pull request Jan 11, 2024
In gocardless#522 the ability to
enable gaplock protection manually was removed, likely assuming there
was no need for this since it's enabled by default when using an adapter
that is named `mysql*`, however we use
https://github.com/trilogy-libraries/trilogy which has been gaining some
traction in the community and this change now leaves us without a way to
enable the functionality.

I think it makes most sense to enable the functionality by default when
using this new adapter as well, and keep config requirements minimal, so
I added a little check for that adapter name as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants