mirror of
https://github.com/run-llama/llama_deploy.git
synced 2026-08-24 21:01:26 -04:00
[PR #397] [MERGED] fix: avoid early exit of RabbitMQ consumer #438
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
📋 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:
main← Head:massi/393📝 Commits (10+)
755bdeeadd e2e test running a workflow2ccadebcosmetics5a75f1duse topic everywhere77e8048keep the consumer alivee90fd80add multi-control plane tests4419619proper manage cancellation and cancel task in tests0d2819fincrease coverage6203f0drelax assertion046347ewait for the process to finish39aa04arun queue on a different tcp port📊 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:
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 :)topicas dictated from the control plane, solving #362. Also added proper e2e tests around this🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.