[PR #397] [MERGED] fix: avoid early exit of RabbitMQ consumer #438

Closed
opened 2026-02-16 01:17:21 -05:00 by yindo · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/run-llama/llama_deploy/pull/397
Author: @masci
Created: 12/6/2024
Status: Merged
Merged: 12/7/2024
Merged by: @masci

Base: mainHead: massi/393


📝 Commits (10+)

📊 Changes

7 files changed (+195 additions, -48 deletions)

View changed files

📝 e2e_tests/message_queues/message_queue_kafka/conftest.py (+5 -4)
📝 e2e_tests/message_queues/message_queue_rabbitmq/conftest.py (+80 -0)
📝 e2e_tests/message_queues/message_queue_rabbitmq/docker-compose.yml (+3 -8)
📝 e2e_tests/message_queues/message_queue_rabbitmq/test_message_queue.py (+19 -0)
📝 e2e_tests/message_queues/message_queue_simple/test_message_queue.py (+1 -1)
📝 llama_deploy/message_queues/rabbitmq.py (+41 -28)
📝 tests/message_queues/test_rabbitmq.py (+46 -7)

📄 Description

Fixes #393 #362

Problem found: the message consumer task was exiting after the first message received. The bug didn't surface probably because most of our tests are happy with a roundtrip of one message only

In this PR:

  • Added await asyncio.Future() to keep the message consumer around. I don't particularly like this fix but it seems to be the idiomatic way to do it with Rabbit from what I found online. FWIW Claude suggested the same fix when prompted so there must be some statistical corroboration behind :)
  • Made the message queue use the topic as dictated from the control plane, solving #362. Also added proper e2e tests around this
  • Updated type annotations

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/run-llama/llama_deploy/pull/397 **Author:** [@masci](https://github.com/masci) **Created:** 12/6/2024 **Status:** ✅ Merged **Merged:** 12/7/2024 **Merged by:** [@masci](https://github.com/masci) **Base:** `main` ← **Head:** `massi/393` --- ### 📝 Commits (10+) - [`755bdee`](https://github.com/run-llama/llama_deploy/commit/755bdeea177bd2426fcfdf924cb5504b977c4898) add e2e test running a workflow - [`2ccadeb`](https://github.com/run-llama/llama_deploy/commit/2ccadeb8e617e6e5e9bbd7049f65ced43c06e20d) cosmetics - [`5a75f1d`](https://github.com/run-llama/llama_deploy/commit/5a75f1dc56c93de2dbb927a6617480566948e3c9) use topic everywhere - [`77e8048`](https://github.com/run-llama/llama_deploy/commit/77e80480fb4cd2bff6042c1b90aea4b23cb35fc5) keep the consumer alive - [`e90fd80`](https://github.com/run-llama/llama_deploy/commit/e90fd8058b137e6468c826a29cae9f73307777cb) add multi-control plane tests - [`4419619`](https://github.com/run-llama/llama_deploy/commit/44196195a6935669c88871f787044232dcd64423) proper manage cancellation and cancel task in tests - [`0d2819f`](https://github.com/run-llama/llama_deploy/commit/0d2819fb28a2174e23df7c6a30914e5696d4bff4) increase coverage - [`6203f0d`](https://github.com/run-llama/llama_deploy/commit/6203f0d35d0380928ec12a15169afefe325b67e9) relax assertion - [`046347e`](https://github.com/run-llama/llama_deploy/commit/046347ed1a1f8e2e9d027996acd87a760885089b) wait for the process to finish - [`39aa04a`](https://github.com/run-llama/llama_deploy/commit/39aa04a69c505fc47178c62563301259e39423ed) run queue on a different tcp port ### 📊 Changes **7 files changed** (+195 additions, -48 deletions) <details> <summary>View changed files</summary> 📝 `e2e_tests/message_queues/message_queue_kafka/conftest.py` (+5 -4) 📝 `e2e_tests/message_queues/message_queue_rabbitmq/conftest.py` (+80 -0) 📝 `e2e_tests/message_queues/message_queue_rabbitmq/docker-compose.yml` (+3 -8) 📝 `e2e_tests/message_queues/message_queue_rabbitmq/test_message_queue.py` (+19 -0) 📝 `e2e_tests/message_queues/message_queue_simple/test_message_queue.py` (+1 -1) 📝 `llama_deploy/message_queues/rabbitmq.py` (+41 -28) 📝 `tests/message_queues/test_rabbitmq.py` (+46 -7) </details> ### 📄 Description Fixes #393 #362 Problem found: the message consumer task was exiting after the first message received. The bug didn't surface probably because most of our tests are happy with a roundtrip of one message only In this PR: - Added `await asyncio.Future()` to keep the message consumer around. I don't particularly like this fix but it seems to be the idiomatic way to do it with Rabbit from what I found online. FWIW Claude suggested the same fix when prompted so there must be some statistical corroboration behind :) - Made the message queue use the `topic` as dictated from the control plane, solving #362. Also added proper e2e tests around this - Updated type annotations --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
yindo added the pull-request label 2026-02-16 01:17:21 -05:00
yindo closed this issue 2026-02-16 01:17:21 -05:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: run-llama/llama_deploy#438