-
Notifications
You must be signed in to change notification settings - Fork 5
Use Data Section by Default instead of AMQPValue for the message creation #63
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
Conversation
Signed-off-by: Gabriele Santomaggio <[email protected]>
Signed-off-by: Gabriele Santomaggio <[email protected]>
Signed-off-by: Gabriele Santomaggio <[email protected]>
Signed-off-by: Gabriele Santomaggio <[email protected]>
Signed-off-by: Gabriele Santomaggio <[email protected]>
Signed-off-by: Gabriele Santomaggio <[email protected]>
Signed-off-by: Gabriele Santomaggio <[email protected]>
Signed-off-by: Gabriele Santomaggio <[email protected]>
Signed-off-by: Gabriele Santomaggio <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR changes the default message body section from AmqpValue to Data by enforcing the use of byte representations and updates tests and examples accordingly. Key changes include updating message body conversion using the new Converter utility, enforcing a new inferred flag check in publishers, and modifying management message construction.
Reviewed Changes
Copilot reviewed 22 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
tests/test_consumer.py | Updated message assertion to use Converter for byte-to-string conversion. |
tests/test_connection.py | Added assertions to verify existence of SSL certificate files. |
tests/test_amqp_091.py | Updated publishing and consuming logic to use Converter for string and byte conversions. |
tests/conftest.py | Simplified annotation dictionary construction. |
rabbitmq_amqp_python_client/utils.py | Introduced Converter class for handling string/byte conversions. |
rabbitmq_amqp_python_client/qpid/proton/_message.py | Added an explicit inferred parameter with minor refactoring of the inferred property. |
rabbitmq_amqp_python_client/qpid/proton/_exceptions.py | Added a new ArgumentOutOfRangeException for parameter validation. |
rabbitmq_amqp_python_client/publisher.py | Implemented new validations ensuring message.body is bytes and inferred is True. |
rabbitmq_amqp_python_client/management.py | Modified message creation to set inferred=False for management messages. |
rabbitmq_amqp_python_client/init.py | Exported the Converter from the client package. |
pyproject.toml | Reorganized dependencies with test and development groups. |
examples/tls/tls_example.py | Updated TLS example to use Converter in message handling. |
examples/streams/example_with_streams.py | Updated message creation to use the Converter utility. |
examples/reconnection/reconnection_example.py | Updated message publishing to use Converter for byte conversions. |
examples/oauth/oAuth2.py | Updated oAuth example to use Converter for message creation and message logging. |
examples/getting_started/getting_started.py | Updated basic example to convert messages using Converter. |
examples/README.md | Fixed reference to the oAuth2 example. |
README.md | Updated messaging example to use Converter and corrected oAuth2 example reference. |
Files not reviewed (2)
- .ci/ubuntu/gha-setup.sh: Language not supported
- Makefile: Language not supported
@@ -134,6 +134,7 @@ def _request( | |||
amq_message = Message( | |||
id=id, | |||
body=body, | |||
inferred=False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The management code creates a Message with inferred set to False, but the publisher now enforces that message.inferred must be True. Consider updating the management message creation to set inferred=True or adjust the publisher's validation for management messages.
inferred=False, | |
inferred=True, |
Copilot uses AI. Check for mistakes.
based on: rabbitmq/rabbitmq-amqp-python-client#63 Signed-off-by: Gabriele Santomaggio <[email protected]>
based on: rabbitmq/rabbitmq-amqp-python-client#63 Signed-off-by: Gabriele Santomaggio <[email protected]>
0x75
) instead of AmqpValue (0x77
)