Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for custom response exchange #104

Merged
merged 2 commits into from
Apr 21, 2024

Conversation

jgrasboeck
Copy link

Hi @dapper91,

First of all, I want to thank you for creating this extensive package!

We are currently adopting your library into our application. As we are using rabbit mq for communication, we use aio_pika as backend.
To integrate the rpc messaging smoothly into the rest of our communication architecture, we would need to be able to specify a custom exchange for the responses plus a custom routing key.

Here's the proposed adjustment of the aio_pika Executor.

@@ -40,17 +53,20 @@ def dispatcher(self) -> pjrpc.server.AsyncDispatcher:

return self._dispatcher

async def start(self, queue_args: Optional[Dict[str, Any]] = None) -> None:
async def start(self, queue_args: Optional[Dict[str, Any]] = None, exchange_args: Optional[Dict[str, Any]] = None) -> None:
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is too long. Please split it to be less than 120 characters.

),
routing_key=reply_to,
)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, remove extra whitespace

correlation_id=message.correlation_id,
content_type=pjrpc.common.DEFAULT_CONTENT_TYPE,
),
routing_key=self._tx_routing_key if self._tx_routing_key else reply_to,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be necessary to check that self._tx_routing_key is not None or reply_to is not None otherwise routing_key will be None which is forbidden.

Copy link
Author

@jgrasboeck jgrasboeck Apr 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct, thx! Fixed it by using default routing key "" in case neither reply_to or tx_routing_key is given. This could be usesfull on cases where a fanout exchange is used as target.

@dapper91
Copy link
Owner

@jgrasboeck Hi,

Thanks for your feedback and contribution!

@jgrasboeck
Copy link
Author

@dapper91 You're very welcome! Anything else I should adjust?

@jgrasboeck
Copy link
Author

jgrasboeck commented Apr 21, 2024

@dapper91 bump

@dapper91 dapper91 merged commit daafe44 into dapper91:dev Apr 21, 2024
0 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants