Add Dead Letter Queue class, integrate into heartbeat #918
Conversation
Leftover copypasta from sentry tests
- warn when we have a bug in the queue with no items - various debug messages
leplatrem
left a comment
leplatrem
left a comment
There was a problem hiding this comment.
Thank you Graham for bringing this to another level 🙏 💯
Co-authored-by: Mathieu Leplatre <[email protected]>
|
One more big though I'm having about the architecture of this PR: We've been calling this a (singular) queue. But almost feels like we're building a queue manager. Like to me, what we return from |
In my head, this is a very simple partitioned queue. We have a single queue where things are going, but blocking only occurs within the partition (per bug). These terms would be more well defined if we were using an actual queue service with partitions and compute nodes. |
- turn back into a method - allow callers to pass a bug_id to filter size by bug
Return iterator of items from get() Return dict of bug id, items from get_all() Return backend.get_all() from retrieve (instead of flat list)
leplatrem
left a comment
leplatrem
left a comment
There was a problem hiding this comment.
I think this is great for a first iteration.
The retries field seems unused. We can drop it if we don't have any API to manipulate it.
Ideally I would like us to tackle these two in follow-ups:
alexcottner
left a comment
alexcottner
left a comment
There was a problem hiding this comment.
This is looking really good. Left comment about an additional possible test scenario.
…n't match our schema
This also means we're marking a webhook event's time property as not optional
This allows us to fetch the item identifiers in the queue without loading the items into memory
Also, document QueueItemRetrievalError
Built on the groundwork of #903 (cheers @leplatrem, when I squash this I'll make sure you're a coauthor), this PR adds the classes that allow us to manage our Dead Letter Queue.
We included a basic heartbeat check in this PR to assert that we are able to read / write from the queue. I removed the integration with the app (in the
/bugzilla_webhookroute) for now. The idea is that we'll deploy this change first to validate that we can connect to the directory we're mounting on the infra side. Then, we'll follow on with how we integrate it into the/bugzilla_webhookroute.