fix(sentry-apps): default flush to false for sentry app models #107309
Conversation
github-actions
bot
added
the
Scope: Backend
|
There's a test failing that I'm not sure what the fix is yet |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
|
|
||
| @control_silo_model | ||
| class SentryApp(ParanoidModel, HasApiScopes, Model): | ||
| default_flush = False |
There was a problem hiding this comment.
default_flush attribute on SentryApp is unused
Low Severity
The default_flush = False attribute on SentryApp class is dead code. Unlike SentryAppInstallation which inherits from ReplicatedControlModel (which uses self.default_flush), SentryApp inherits from ParanoidModel, HasApiScopes, Model - none of which reference this attribute. Additionally, SentryApp has explicit implementations of save(), update(), and delete() that call outbox_context(..., flush=False) directly without ever reading self.default_flush. The attribute provides no functionality and may mislead developers into thinking it affects behavior.
| def delete(self, *args, **kwargs): | ||
| with outbox_context(transaction.atomic(using=router.db_for_write(SentryAppInstallation))): | ||
| with outbox_context( | ||
| transaction.atomic(using=router.db_for_write(SentryAppInstallation)), flush=False |
There was a problem hiding this comment.
If default_flush=False do we also need flush=False here? You could reference self.default_flush here if you wanted to be explicit 🤷
There was a problem hiding this comment.
I guess I was doing this because the update and save functions in this model also do it, but we can change it
2 participants
Sentry app and install outboxes for deletion are currently flushed synchronously, they should be async to prevent db contention when the deletion outboxes are processed.