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

Investigate on the AMQP external clients to use #1

Closed
DanielePalaia opened this issue Dec 4, 2024 · 4 comments · Fixed by #10
Closed

Investigate on the AMQP external clients to use #1

DanielePalaia opened this issue Dec 4, 2024 · 4 comments · Fixed by #10
Assignees

Comments

@DanielePalaia
Copy link
Contributor

DanielePalaia commented Dec 4, 2024

Our RabbitMQ AMQP clients use an internal AMQP 1.0 lib to send an receive messages from the broker.

In case of Python there are two possibilities:

This is the client our RabbitMQ Java client is using.
Qpid proton for Python can be used directly as library.
It looks like the library is using some C code internally:
https://github.com/apache/qpid-proton/tree/main/python

I made a few tests (Connection, Declaration of Queues/Exchanges) and even if the library is not always well documented I was able to let it works for these use cases so I think we can proceed using it.

  • Reuse the code of the Azure Service bus:

We could reuse the code in:

https://github.com/Azure/azure-sdk-for-python/tree/main/sdk/servicebus/azure-servicebus/azure/servicebus/_pyamqp

This was a rework of this original library:

https://github.com/Azure/azure-uamqp-python

That will be deprecated soon: Azure/azure-uamqp-python#374

This is the approach used by rstream (but just for the serialization part of the protocol).

While the original library was Cython base the one moved in the ServiceBus seems like to be "pure python"

The disadvantage is that it is not possible to import the library directly in the project but we will need to copy/paste the folder code in our project and possibly maintain that. Also I made a few tests and I wasn't able to fully let it work for our use case.

@Gsantomaggio @lukebakken : What do you think? I would probably proceed to include and use the qpid-proton lib if for you is fine. Do you see any drawbacks/alternatives we can use?

@DanielePalaia DanielePalaia added enhancement New feature or request exploration and removed enhancement New feature or request labels Dec 4, 2024
@Gsantomaggio
Copy link
Member

I vote for Qpid.

I'm not too fond of the idea of importing the whole https://github.com/Azure/azure-sdk-for-python/tree/main/sdk/servicebus/azure-servicebus/azure/servicebus/_pyamqp .

For stream, we imported only the codec, which is a limited part.

@DanielePalaia
Copy link
Contributor Author

Decision taken!

@DanielePalaia DanielePalaia changed the title Investigate on the AMQP clients to use Investigate on the AMQP external clients to use Dec 5, 2024
@DanielePalaia DanielePalaia reopened this Jan 2, 2025
@DanielePalaia
Copy link
Contributor Author

DanielePalaia commented Jan 2, 2025

This should be re-evaluated.

After integrating the qpid-proton library here: #10 and implementing the basic operations we discovered a bug in the library during the encoding of a null body amqp message needed in order to do some deletions: DELETE of queues, exchanges, purge of a queue ecc ecc.

The issue is explained on a e-mail I sent to the qpid-proton team which unfortunately didn't receive any response:
https://lists.apache.org/[email protected]

After an initial investigation on this library we discover that it can be complicated to provide a fix ourself, because the encoding/decoding functionality is implemented inside a very complex C code layer which could be complex to modify/link and import ourself, also we would have in this case to maintain three different libs: The C layer doing the coding and decoding, the qpid proton wrapping python library and this library as well.

We are reconsidering investigating:

https://github.com/Azure/azure-sdk-for-python/tree/main/sdk/servicebus/azure-servicebus/azure/servicebus/_pyamqp

but also this code seems to have several issues to be used for our purposes. This code seems implemented in order to work with their azure-service bus scenario and not to be offered as a generic library, and it is also having the same serialization issue discovered in qpid-proton (even tough easier to implement).

In general some modifications of the code seem needed and it is not sure yet it will really work for our use-case

@DanielePalaia
Copy link
Contributor Author

I will write the last final summary on this and how we solve that.

To summarize:

  • Both library have some bugs/limitations for our use case. Qpid-proton seems working good except for the NULL body encoding, which may be too difficult to fix directly in the library because is implemented in a quite comple C code layer
  • The Azure amqp code seems very specific for the azure service-bus. The code here: https://github.com/Azure/azure-sdk-for-python/tree/main/sdk/servicebus/azure-servicebus/azure/servicebus/_pyamqp is a rework of this library: https://github.com/Azure/azure-uamqp-python which will be deprecated soon. Having studied this library unfortunately if we want to use it we need to rewrite several parts of it. The library seems creating different sessions for both Senders and Receivers plus there are some missing points like the names and source/target addresses of Senders and Receivers and it is not able to create a proper bi-directional link. After several days of investigation in the code (discovering several missing points) and wireshark packets we decided to discard it

As final decision for the moment we decided to consider our case a special case for the qpid-proton library. Just the python session of the library has been imported and we will provide the fix after the C layer completed the serialization. This special case will be managed just when we are targetting the /management endpoint so no side-effect should be created.

The integration is happening on this PR: #10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants