Skip to content

switch dashboard stats queries to use new bulk DB session#4625

Draft
spatel033 wants to merge 3 commits intomainfrom
dashboard-queries-bulk-db
Draft

switch dashboard stats queries to use new bulk DB session#4625
spatel033 wants to merge 3 commits intomainfrom
dashboard-queries-bulk-db

Conversation

@spatel033
Copy link
Contributor

@spatel033 spatel033 commented Nov 11, 2025

This change routes dashboard stats queries to the new 'bulk' database via the session_bulk. We are starting with the dashboards as the first real-world test of the replica connection. This is a low-risk change that can be easily rolled back if users see any issues.
This PR will be deployed after initial connectivity test: #4624. More details on this card.

@spatel033 spatel033 changed the title Dashboard queries bulk db switch dashboard queries to use new bulk DB session Nov 11, 2025
@risicle
Copy link
Member

risicle commented Dec 2, 2025

There are two tests of fetch_notification_status_for_service_for_today_and_7_previous_days in tests/app/dao/test_fact_notification_status_dao.py, I'd be a lot more confident if you'd parametrize them in the same way I did in tests/app/dao/test_fact_billing_dao.py in 017321f#diff-6bca20a139110156cad191771c83758f64fd3aaa68e3b39d71ca590d384ee7c7 so that it checks that the function works whether we're using the default session or bulk session. Note how for these tests I just accessed db.session or db.session_bulk` directly instead of worrying about a special fixture for the bulk session (I'm not entirely sure it's necessary TBH)

Obviously you can't add the QueryRecorder stuff to check where the queries are actually going (yet?)

Add notify_db_session_bulk fixture to yield _notify_db.session_bulk for tests that need to run against the bulk/replica database. Ensures cleanup after each test by invoking _clean_database(_notify_db), keeping behaviour consistent with notify_db_session.
…uery

Replace flask_sqlalchemy's get_recorded_queries with SQLAlchemy event listeners that capture the bind_key for each engine. Add QueryInfo dataclass with statement, parameters, and bind_key fields. This enables tests to verify that queries are being routed to the correct database when using db.session_bulk.
This change routes dashboard queries to the new 'bulk' database via the . We are starting with the dashboards as the first real-world test of the replica connection. This is a low-risk change that can be easily rolled back if users see any issues.
@spatel033 spatel033 force-pushed the dashboard-queries-bulk-db branch from 5b0f1fa to dbc34ed Compare December 4, 2025 13:12
@spatel033 spatel033 changed the title switch dashboard queries to use new bulk DB session switch dashboard stats queries to use new bulk DB session Dec 4, 2025
@spatel033 spatel033 marked this pull request as ready for review December 4, 2025 13:21
@spatel033 spatel033 marked this pull request as draft December 10, 2025 15:07
@risicle
Copy link
Member

risicle commented Dec 11, 2025

We probably need to adapt this to chop up the queries if we want to run it on the replica.

@risicle
Copy link
Member

risicle commented Dec 11, 2025

See #4669

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.

2 participants