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

Open
#47,815 0 comments 1 reaction 1 assignee Claimed by @j7nw4r View on GitHub
Event Hubs Messaging
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.