fix(hc): Prevent exception from siloed_atomic #52326
Conversation
An exception rises from `router.db_for_write` if the database doesn't exist in the environment, as observed with `SENTRY_USE_SILOS` set to true. Introduce a custom exception type to mark this condition, and catch it when setting the atomic impl. This should be equivalent behavior, as we only care about whether "missing" database is equal to `using` (which a null value won't be).
github-actions
bot
added
the
Scope: Backend
|
Related to #51967. @GabeVillalobos Let me know if you agree with my take on the error condition. You should be able to reproduce it by running |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #52326 +/- ##
==========================================
+ Coverage 77.72% 79.32% +1.59%
==========================================
Files 4883 4909 +26
Lines 205442 205768 +326
Branches 35126 35168 +42
==========================================
+ Hits 159681 163221 +3540
+ Misses 40650 37561 -3089
+ Partials 5111 4986 -125
|
Resolve some failures in `tests/getsentry/db/test_router.py` where ValueError is expected. We can make the tests narrower after this change is pushed up to getsentry.
| using == router.db_for_write(ControlOutbox) | ||
| and SiloMode.get_current_mode() != SiloMode.CONTROL | ||
| ): | ||
| elif using == control_db and SiloMode.get_current_mode() != SiloMode.CONTROL: | ||
| raise MismatchedSiloTransactionError( | ||
| f"Cannot use transaction.atomic({using}) in Control Mode" |
There was a problem hiding this comment.
@RyanSkonnord unsure if these error copies need to be corrected.
| f"Cannot use transaction.atomic({using}) in Control Mode" | |
| f"Cannot use transaction.atomic({using}) in Region Mode" |
There was a problem hiding this comment.
Yes, seems like those should be switched. Or perhaps should say "except in control mode". @GabeVillalobos?
There was a problem hiding this comment.
Great. But it's preexisting, so let's handle it in a separate PR.
An exception rises from
router.db_for_writeif the database doesn't exist in the environment, as observed withSENTRY_USE_SILOSset to true. Introduce a custom exception type to mark this condition, and catch it when setting the atomic impl. This should be equivalent behavior, as we only care about whether "missing" database is equal tousing(which a null value won't be).