Better handling for described encoding#45878
Conversation
There was a problem hiding this comment.
Pull request overview
Improves AMQP described-type decoding in the Event Hubs pyamqp stack by preserving the descriptor on decoded values and fixing decoding for arrays whose element type is described.
Changes:
- Adds described-value wrapper types (e.g.,
DescribedList) to retain a.descriptorattribute on decoded values. - Updates array decoding to correctly handle arrays of described element types (small + large).
- Extends unit tests to cover described values and arrays of described values.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
sdk/eventhub/azure-eventhub/tests/pyamqp_tests/unittest/test_decode.py |
Adds tests for described decoding and described arrays. |
sdk/eventhub/azure-eventhub/azure/eventhub/_pyamqp/described.py |
Introduces wrapper subclasses used to attach .descriptor to decoded values. |
sdk/eventhub/azure-eventhub/azure/eventhub/_pyamqp/_decode.py |
Updates described decoding and array decoding; introduces constructor→wrapper mapping. |
Comments suppressed due to low confidence (1)
sdk/eventhub/azure-eventhub/azure/eventhub/_pyamqp/_decode.py:325
- In _decode_described_array, _COMPOSITES maps descriptor -> string (e.g., "accepted"), but the code uses cast(int, _COMPOSITES[descriptor]) and stores it in composite_type. This cast is incorrect/misleading for type checking and makes the variable name confusing. Update the cast/type annotation (likely str) and/or rename the variable to reflect it holds a composite name/key, not an int type code.
try:
composite_type = cast(int, _COMPOSITES[descriptor])
return buffer, {composite_type: value}
except KeyError:
| try: | ||
| value = _DESCR_BY_CONSTRUCTOR[tp](value, descriptor=descriptor) | ||
| except KeyError: | ||
| pass |
There was a problem hiding this comment.
This only attaches .descriptor for constructors listed in _DESCR_BY_CONSTRUCTOR; for any other described underlying type (e.g., uuid/decimal128/bool/null), the descriptor is still dropped via the KeyError path. If the intent is to preserve the descriptor for all described values (per PR description), consider adding a generic fallback wrapper that stores (descriptor, value) when a specialized subclass wrapper is not available.
sdk/eventhub/azure-eventhub/azure/eventhub/_pyamqp/described.py
Outdated
Show resolved
Hide resolved
sdk/eventhub/azure-eventhub/tests/pyamqp_tests/unittest/test_decode.py
Outdated
Show resolved
Hide resolved
|
/azp run python - eventhub - tests |
Description
This pr keeps descriptor as "descriptor" attribute.
All SDK Contribution checklist: