Feat: AMQP module #68
Conversation
bradleytrf
requested review from
debajyoti-truefoundry,
chiragjn and
nikp1172
as code owners
|
tested on google rabbitmq
|
|
| await channel.default_exchange.publish( | ||
| Message(body=serialized_output_message), routing_key=self._queue_name |
There was a problem hiding this comment.
I see we have restricted ourselves to default exchange and re-using queue name as routing key
Shouldn't the exchange name and routing key be distinct inputs?
Maybe not today but some day in future?
afaik, for publishing we don't need queue name, just the exchange + routing key
What queue is bound to what exchange and routing key is not needed by us when publishing
See: https://aio-pika.readthedocs.io/en/latest/rabbitmq-tutorial/4-routing.html#bindings
| await channel.default_exchange.publish( | ||
| Message(body=serialized_input_message), routing_key=self._queue_name | ||
| ) |
There was a problem hiding this comment.
exchange name and routing key should be configurable
Maybe not today but some day in future?
Same reason as this: #68 (comment)
We don't need the queue name directly to publish
There was a problem hiding this comment.
Another question - Is it always guaranteed that a routing key = X always delivers to a queue named X?
| url: constr( | ||
| regex=r"^(?:amqp|amqps):\/\/(?:([^:/?#\s]+)(?::([^@/?#\s]+))?@)?([^/?#\s]+)(?::(\d+))?\/?([^?#\s]*)?(?:\?(.*))?$" | ||
| ) | ||
| queue_name: str |
There was a problem hiding this comment.
Ideally this should be
| queue_name: str | |
| exchange_name: str | |
| routing_key: str |
| self, serialized_output_message: bytes, request_id: Optional[str] | ||
| ): | ||
| channel = await self._get_channel() | ||
| await self._get_queue() |
https://truefoundry.atlassian.net/browse/TT-6142