-
Notifications
You must be signed in to change notification settings - Fork 2
feat(utils): Allow for explicit use of database replicas #95
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
Conversation
- Prevents conflicts with other systems - Less unused code, yay
18e8d28 to
e72f1e5
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #95 +/- ##
==========================================
+ Coverage 94.08% 94.51% +0.43%
==========================================
Files 73 75 +2
Lines 2232 2408 +176
==========================================
+ Hits 2100 2276 +176
Misses 132 132 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
e72f1e5 to
3e4ec91
Compare
0713456 to
c4d1583
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, looks great. Added a couple questions of moderate to low impact. Would like to see this tested in threaded Gunicorn to understand whether we need to use thread-local storage for the sequential replica cache. Besides that, interested to see if there's a performance effect on high-volume queries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to export Prometheus metrics for this?
e.g. flagsmith_db_chosen_replica_count labeled by status and replica_connection_name.
I'd be happy to do it given an example, and if it doesn't extend much of the scope here. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 Looking forward to seeing this used in Core.
Contributes to Flagsmith/flagsmith#5814.
This is meant to replace the
PrimaryReplicaRouterfunctionality in the core API.Changes
Introduces a utility
using_database_replicathat wraps any Django model manager binding it to a database replica according tosettings.REPLICA_READ_STRATEGY.Tests
Unit tests.