Azure / Azure/azure-sdk-for-python
[Event Hubs] set_message_partition_key overwrites the AMQP message header, dropping caller-set TTL/priority/durable on partition-keyed sends
- Dominant language
- Python
- Stars
- 5.6k
- Forks
- 3.4k
- Avg merge
- 1d 21h
- Merged PRs (30d)
- 193
Description
## Summary
Sending an `EventData` (or `AmqpAnnotatedMessage`) that carries a partition key silently drops any AMQP message-header fields the caller set: `time_to_live`, `priority`, `first_acquirer`, `delivery_count`, and an explicit `durable = False`. The loss happens on every partition-keyed send path (single `send`, `_set_partition_key`, and a partition-keyed `EventDataBatch.add`) and is identical on both the `uamqp` and the modern default `pyamqp` transports.
## Motivation
In the outgoing pipeline, `to_outgoing_amqp_message` builds the AMQP header by copying all five header fields off the user's message (`_uamqp_transport.py:155-165`, `_pyamqp_transport.py:98-108`). `set_message_partition_key` then runs on that already-built message and replaces `.header` wholesale with a fresh header whose only non-default is `durable=True`, without ever reading the existing header:
- uamqp `_transport/_uamqp_transport.py:459-462`: `header = MessageHeader(); header.durable = True; ... message.header = header`
- pyamqp `_transport/_pyamqp_transport.py:394-395`: `header = Header(durable=True); return message._replace(message_annotations=annotations, header=header)`
Concrete failure: a caller sets `ed.raw_amqp_message.header.time_to_live = 60000` and sends with a `partition_key`. `to_outgoing_amqp_message` builds a header carrying `ttl=60000`; `set_message_partition_key` then overwrites it with a `durable=True`-only header and the per-message TTL (and any priority/first_acquirer/delivery_count/explicit `durable=False`) is dropped on the wire. The empty-batch-envelope path (`EventDataBatch.__init__`) is unaffected because there is no user header yet; the loss is on the per-event `add` and single-send paths.
The forced `durable=True` coupling is undocumented. It traces back unchanged to the 2019 monorepo migration (#4764) with no linked issue or comment explaining why partition-keyed messages must be durable, and it also silently overrides a caller's explicit `durable=False`. The only nearby `TODO` (`_uamqp_transport.py:457`) concerns the annotation key type, not the header overwrite.
## Proposal
Stop reconstructing the header in `set_message_partition_key`. Mutate the field on the existing header (`message.header.durable = True`, allocating a header only when one is absent), or set the partition-key annotation without touching the header at all, so caller-set `time_to_live`/`priority`/`first_acquirer`/`delivery_count` and an explicit `durable` value survive. Apply the same change to both transports. Before altering the `durable=True` behavior itself, confirm whether forcing durable on partition-keyed sends is an intentional Event Hubs default; the fix should preserve that behavior while ending the clobber of the other header fields.
## Validation
Add unit coverage on both transports asserting that a message with `time_to_live` (and `priority`) set retains those fields after `set_message_partition_key`, and that an explicit `durable = False` is not silently forced to `True`.
Contributor guide
Assessment
This issue has not been assessed yet.