feat(python): add user header and origin timestamp support#3613
feat(python): add user header and origin timestamp support#3613jiengup wants to merge 2 commits into
Conversation
|
one additional request from my side: please update python examples to include a code that would show how to use iggy message user headers. there is one for rust already. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3613 +/- ##
============================================
- Coverage 73.47% 73.25% -0.23%
Complexity 937 937
============================================
Files 1286 1287 +1
Lines 141019 141307 +288
Branches 116908 116658 -250
============================================
- Hits 103619 103511 -108
- Misses 34175 34519 +344
- Partials 3225 3277 +52
🚀 New features to boost your workflow:
|
That will be included when the PR is ready. |
a5a6357 to
6d29d49
Compare
|
I was thinking that maybe we should change the signature of the headers from This would allow us to have two advantages: users that do not have much experience/do not require explicit control can use What do you guys think? |
I think it's a good idea and well aligned with the first point worth discussing in my PR. I'll push a new patch soon. |
|
@slbotbm for me this sounds fine, however one thing comes to my mind: will it have any impact on performance? |
Actually, I think additional typed The only overhead we introduce is the iteration of |
|
alright, then don't worry about it now. |
0fdd454 to
8daf546
Compare
|
Hi, I just reflected the change and merged all the commits into a final one. The PR message is also updated. Instead of a plain-only Please have a look at it when you are free. |
8daf546 to
90b153d
Compare
| await produce_messages(client) | ||
|
|
||
|
|
||
| async def init_system(client: IggyClient): |
There was a problem hiding this comment.
Current system catches stream creation and topic creation independently, logs errors, and continues with an invalid system. Unify the try/catches and exit if stream/topic is not created
There was a problem hiding this comment.
I don't get this point.
The message header examples are implemented accroding to the existing getting-started example.
There was a problem hiding this comment.
In common.py, in the init_system function, topic creation will be tried even if the stream creation fails. Since topics reside inside streams in iggy, it will not be created and will fail. Better to change the code so that topic should only be created if the stream exists/is created.
|
/author |
Add user headers to SendMessage and ReceiveMessage, expose origin timestamp, and fix Docker test infrastructure: - explicit typed header and plain Python header support - type transformation and validation across Rust and Python - user header usage examples - minor fix: Docker test config file
to_scalar_dict Split message-headers example into plain-headers and typed-headers variants sharing logic via common.py. Rename UserHeaders.to_plain() to to_scalar_dict() to better describe the returned dict type.
90b153d to
209ecd7
Compare
|
/ready |
slbotbm
left a comment
There was a problem hiding this comment.
Some more comments:
- You have set the type of user headers in some places to [Any, Any], which makes any type checker incapable of catching mistakes. Would be better to explicitly declare the types.
| pub fn to_scalar_dict<'a>(slf: &Bound<'a, Self>) -> PyResult<Bound<'a, PyDict>> { | ||
| let py = slf.py(); | ||
| let dict = slf.as_any().cast::<PyDict>()?; | ||
| let headers = py_user_headers_to_rust(py, dict)?; | ||
| rust_user_headers_to_plain_py(py, headers) | ||
| } |
There was a problem hiding this comment.
This function is not lossless - if the user supplies UnsignedInt8(1) and UnsignedInt16(1), both get converted to python int 1. In this case, the conversion should fail, and be documented properly.
There was a problem hiding this comment.
Also, the current code converts twice before returning the headers back to the user: python typed headers -> rust headers -> python plain headers. It would be preferable to do a direct conversion python typed header -> python plain header. TryFrom can be defined and utilized.
There was a problem hiding this comment.
if the user supplies
UnsignedInt8(1)andUnsignedInt16(1)
I don't get this.
Lossless here means Rust type conversion into Python type should not cost any data loss (e.g. uint32 to py Int because Python can represent any large int).
You mean we should reject any coexisting UnsignedInt8(1) and UnsignedInt16(1) or we should reject UnsignedInt16(-1)? I think the former is not reasonable.
There was a problem hiding this comment.
Let me think a little about this and get back to you
| HTTP_CREATED = 201 | ||
| JsonValue: TypeAlias = ( | ||
| None | bool | int | float | str | list["JsonValue"] | dict[str, "JsonValue"] | ||
| ) |
There was a problem hiding this comment.
These are never used. Can be removed.
There was a problem hiding this comment.
The logging setup in the tests you have defined aren't really useful.
| typed_key = HeaderKey.UnsignedInt128(42) | ||
| typed_value = HeaderValue.UnsignedInt128(2**96) | ||
| user_headers: dict[HeaderKey, HeaderValue] = { | ||
| typed_key: typed_value, | ||
| HeaderKey.String("float32"): HeaderValue.Float32(1.25), | ||
| } |
There was a problem hiding this comment.
This can be made into one dict. No need to declare extras vars.
| await init_system(client) | ||
| await produce_messages(client, build_plain_headers) |
There was a problem hiding this comment.
Even if init_system fails, this code will not know since init_system logs the error and returns normally.
In produce_messages, the loop termination condition depends on successful sends, but if the send fails, sent_batches is not incremented after a failed send, a persistent configuration, authorization, or initialization failure makes the example run forever. It prints repeated errors instead of terminating with a useful failure.
|
|
||
|
|
||
| def generate_orders() -> Iterator[Order]: | ||
| order_id = 0 |
There was a problem hiding this comment.
In this function, the generated order_ids are not coherent. The current code will effectively produce the following 6 events:
OrderConfirmed(order-0)
OrderRejected(order-0)
OrderCreated(order-3)
OrderConfirmed(order-1)
OrderRejected(order-1)
OrderCreated(order-6)
Add user headers to SendMessage and ReceiveMessage, expose origin timestamp, and fix Docker test infrastructure.
Which issue does this PR address?
Closes #3601 #3612
Rationale
The Python SDK could send and receive message payloads, but it did not expose the typed user headers already carried by the underlying Rust
IggyMessage. This left Python behind the Rust, Node, Go, Java, and C# SDKs for message metadata.What changed?
Python messages can attach user headers and read them back from received messages.
SendMessageacceptsuser_headersand an optional customid, whileReceiveMessageexposesuser_headers()andorigin_timestamp().According to the discussion below, unlike the original proposal in #3601 (which exposed only a plain
dict[str, str | bytes | bool | int | float]), the binding now supports the full Rust header type surface through two exposed classes, plus a plain-scalar convenience layer:HeaderKey/HeaderValue— typed complex enums covering everyHeaderKind:Raw,String,Bool,Int8/16/32/64/128,UnsignedInt8/16/32/64/128,Float32,Float64.UserHeaders— the mapping returned byReceiveMessage.user_headers(). It is a realdictsubclass (dict[HeaderKey, HeaderValue]), so all mapping operations work, and it adds a chainableto_scalar_dict()for the convenient scalar form:message.user_headers().to_scalar_dict().Sending (
plain → Rust)Each key/value pair is converted independently, so typed and plain forms can be freely mixed in one dict (e.g.
{HeaderKey.String("a"): 7, "b": HeaderValue.Bool(True), "c": "plain"}):str):str → String,bytes → Raw,bool → Boolint →the smallest header kind that holds it exactly: non-negative →Uint8/16/32/64/128, negative →Int8/16/32/64/128; values beyond the 128-bit range raiseValueError.float → Float32when the value is exactly representable as f32, otherwiseFloat64(always lossless).OverflowErrorfor out-of-range, e.g.HeaderValue.Int8(300)), and an explicitFloat32whose value overflows f32 (→inf) is rejected withValueError.Receiving (
Rust → plain)Every
HeaderKindmaps losslessly onto a Python scalar (128-bit ints fit Python's arbitrary-precisionint;f32 → f64is exact), so:user_headers()returns typedUserHeaders(dict[HeaderKey, HeaderValue]), orNonewhen the message carries no user headers.UserHeaders.to_scalar_dict()returnsdict[str | bytes | bool | int | float, str | bytes | bool | int | float]— a direct, lossless conversion (no logging); it only raisesValueErroron a genuine decode error of a stored field.Header decode errors from known-but-invalid or unknown semantic kinds surface as
ValueError.SendMessage(id=...)(mapped tou128) is part of the same binding update.Minor Fix
The Python test compose setup starts the server with fresh default root credentials, binds HTTP/TCP/QUIC addresses explicitly, and uses
iggy pingfor the healthcheck instead of the HTTP stats endpoint.Discussion Notes
Choices from the issue discussion, updated to the final implementation:
dictAPI for the common case and explicitHeaderKey/HeaderValuewrapper classes; the two can be mixed per entry.ValueError.Float32/Float64by exact representability instead of alwaysFloat64.HeaderKey), instead of being rejected.ReceiveMessage.user_headers()returnsNonefor no headers.origin_timestamp()is exposed on received messages.API Usage
Sending — plain scalars (common case)
Sending — explicit typed kinds
Sending — mixed typed + plain in one dict
Receiving
Validation / errors
Local Execution
cargo fmt --check --manifest-path foreign/python/Cargo.tomlcargo check --manifest-path foreign/python/Cargo.tomlcargo test --manifest-path foreign/python/Cargo.tomluv run --extra dev ruff check tests/test_message_operations.py tests/test_consumer_group.py.venv/bin/python -m pytest tests/test_message_operations.py::TestMessageOperations::test_invalid_user_headers_are_rejected -qexample/message-headers/typed-headers/consumer.py,example/message-headers/typed-headers/producer.py,example/message-headers/plain-headers/consumer.py,example/message-headers/plain-headers/producer.pyAI Usage
Codex was used to inspect the existing Python, Rust, Node, and Go SDK behavior, implement the Python binding changes, add tests. All the modification was reviewed carefully by the human.