Support encryption context on Message #276
Conversation
BewareMyPower
requested review from
RobertIndie,
Copilot,
merlimat and
shibd
There was a problem hiding this comment.
Pull request overview
This PR adds support for exposing encryption context information on Message objects in the Pulsar Python client. This allows applications to access encryption and compression metadata for messages, particularly useful when handling encrypted messages that fail to decrypt.
Key changes:
- Exposed
EncryptionKeyandEncryptionContextclasses from C++ bindings to Python - Added
encryption_context()method to theMessageclass that returns encryption metadata or None for non-encrypted messages - Enhanced encryption failure tests to verify encryption context information including keys, compression type, batch size, and decryption status
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/message.cc | Exports EncryptionKey and EncryptionContext C++ classes to Python bindings and adds encryption_context() method to Message class |
| pulsar/init.py | Adds Python wrapper classes for EncryptionKey and EncryptionContext with comprehensive documentation and properties for accessing encryption metadata |
| tests/pulsar_test.py | Adds test coverage for encryption_context() on both encrypted and non-encrypted messages, including verification of encryption metadata fields |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for i in range(3): | ||
| msg = reader.read_next(3000) | ||
| self.assertNotEqual(msg.data(), f"msg-{i}".encode()) | ||
| self.assertTrue(len(msg.data()) > 5, f"msg.data() is {msg.data()}") | ||
| verify_undecrypted_message(msg, i) |
There was a problem hiding this comment.
The loop iterates only 3 times but there are 5 messages sent (msg-0, msg-1, msg-2, msg-3, msg-4). This should iterate 5 times using range(5) to verify all messages including the batched messages msg-3 and msg-4.
| Parameters | ||
| ---------- | ||
| key: _pulsar.EncryptionContext |
There was a problem hiding this comment.
The parameter name in the docstring is incorrect. It says "key: _pulsar.EncryptionContext" but should be "context: _pulsar.EncryptionContext" to match the actual parameter name.
| key: _pulsar.EncryptionContext | |
| context: _pulsar.EncryptionContext |
| """ | ||
| return self._message.producer_name() | ||
|
|
||
| def encryption_context(self) -> EncryptionContext: |
There was a problem hiding this comment.
The return type annotation should be Optional[EncryptionContext] instead of EncryptionContext since the method can return None when the message is not encrypted, as documented in the docstring and shown in the implementation.
| def encryption_context(self) -> EncryptionContext: | |
| def encryption_context(self) -> Optional[EncryptionContext]: |
| for i in range(3): | ||
| msg = consumer.receive(3000) | ||
| self.assertNotEqual(msg.data(), f"msg-{i}".encode()) | ||
| self.assertTrue(len(msg.data()) > 5, f"msg.data() is {msg.data()}") | ||
| verify_undecrypted_message(msg, i) |
There was a problem hiding this comment.
The loop iterates only 3 times but there are 5 messages sent (msg-0, msg-1, msg-2, msg-3, msg-4). This should iterate 5 times using range(5) to verify all messages including the batched messages msg-3 and msg-4.
| self.assertTrue(len(key.value) > 0) | ||
| if enc_key is None: | ||
| enc_key = key.value | ||
| else: | ||
| self.assertEqual(key.value, enc_key) | ||
| self.assertEqual(key.metadata, {}) | ||
| self.assertTrue(len(context.param()) > 0) |
There was a problem hiding this comment.
assertTrue(a > b) cannot provide an informative message. Using assertGreater(a, b) instead will give more informative messages.
| self.assertTrue(len(key.value) > 0) | |
| if enc_key is None: | |
| enc_key = key.value | |
| else: | |
| self.assertEqual(key.value, enc_key) | |
| self.assertEqual(key.metadata, {}) | |
| self.assertTrue(len(context.param()) > 0) | |
| self.assertGreater(len(key.value), 0) | |
| if enc_key is None: | |
| enc_key = key.value | |
| else: | |
| self.assertEqual(key.value, enc_key) | |
| self.assertEqual(key.metadata, {}) | |
| self.assertGreater(len(context.param()), 0) |
| else: | ||
| self.assertEqual(key.value, enc_key) | ||
| self.assertEqual(key.metadata, {}) | ||
| self.assertTrue(len(context.param()) > 0) |
There was a problem hiding this comment.
assertTrue(a > b) cannot provide an informative message. Using assertGreater(a, b) instead will give more informative messages.
| self.assertTrue(len(context.param()) > 0) | |
| self.assertGreater(len(context.param()), 0) |
1 participant
No description provided.