[python] avoid etag/match_condition clientName collision with multiple etag headers#10816
[python] avoid etag/match_condition clientName collision with multiple etag headers#10816l0lawrence wants to merge 4 commits into
Conversation
…ion with multiple etag headers When an operation has more than one Azure.Core.eTag-typed header (e.g. Storage's copyFromUrl, which carries both standard If-Match/If-None-Match AND custom x-ms-source-if-match/x-ms-source-if-none-match), PR microsoft#10494's per-parameter conversion renamed every etag-typed header to clientName='etag' or 'match_condition', producing two parameters with the same name on the generated method. Fix: in preprocess update_client, collect all ifMatch/ifNoneMatch candidates per operation, prefer the standard If-Match/If-None-Match wire names for the etag/match_condition slot, and strip etagRole from the non-selected candidates so they retain their natural clientName (e.g. source_if_match). Adds tests/unit/test_preprocess_etag.py covering the standard-only, custom-only, mixed (regression), multi-custom-pair, synthetic-partner, and end-to-end clientName uniqueness scenarios. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
microsoft-github-policy-service
Bot
added
the
emitter:client:python
commit: |
|
All changed packages have been documented.
Show changes
|
|
You can try these changes here
|
msyyc
left a comment
msyyc
left a comment
There was a problem hiding this comment.
A few minor suggestions on the etag-slot picker logic.
| fall back to the first candidate. Returns None if there are no candidates. | ||
| """ | ||
| if not candidates: | ||
| return None |
There was a problem hiding this comment.
Nit: this early-return is redundant — iterating an empty list naturally falls through to return candidates[0], which would raise IndexError. Either drop the guard and rely on the loop (then handle the empty case at the call site), or keep it but note that candidates[0] is unreachable when empty. As-is it's just dead-code-ish.
There was a problem hiding this comment.
Pull request overview
Fixes a regression in the Python HTTP client emitter preprocess step where operations with multiple Azure.Core.eTag-typed headers could end up with duplicate parameter clientNames (etag / match_condition). The change makes slot selection deterministic (prefer standard If-Match / If-None-Match) and demotes non-selected etag headers so they keep their natural names.
Changes:
- Refactors etag header handling in
PreProcessPlugin.update_clientto (a) collect all candidates, (b) pick the promoted pair with a preference for standard wire names, and (c) stripetagRolefrom non-selected headers to avoid renaming collisions. - Adds unit tests covering standard/custom/multiple/custom-only/synthetic-partner scenarios and an end-to-end “no duplicate clientName” check.
- Adds a Chronus changelog entry for the fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/http-client-python/generator/pygen/preprocess/init.py | Refactors etag pairing/selection logic and strips etagRole from non-selected headers to prevent clientName collisions. |
| packages/http-client-python/tests/unit/test_preprocess_etag.py | Adds regression + edge-case tests ensuring correct slot selection and no clientName duplication. |
| .chronus/changes/fix-python-multi-etag-collision-2026-05-27-10-15-00.md | Documents the bug fix for @typespec/http-client-python. |
| def _plugin() -> PreProcessPlugin: | ||
| return PreProcessPlugin( | ||
| output_folder="", | ||
| options={ | ||
| "version-tolerant": True, | ||
| "models-mode": "dpg", | ||
| "show-operations": True, | ||
| "show-send-request": True, | ||
| "builders-visibility": "public", | ||
| }, | ||
| ) |
l0lawrence
requested review from
ChenxiJiang333,
catalinaperalta,
iscai-msft,
lmazuel,
tadelesh and
timotheeguerin
as code owners
Problem
PR #10494 (which added support for custom etag wire names) broke operations that have more than one
Azure.Core.eTag-typed header. The Azure Storage BlobcopyFromUrloperation is a concrete example — it carries:If-Match/If-None-Matchx-ms-source-if-match/x-ms-source-if-none-matchThe emitter in
http.tsstampsetagRole = "ifMatch"/"ifNoneMatch"onto every one of those headers. Then inpreprocess/__init__.py,update_parameterblindly appliesheaders_convert(ETAG_MATCH_DATA)/ETAG_NONE_MATCH_DATAto every parameter with that role — overwritingclientNameto"etag"and"match_condition"on both pairs. The operation ends up with two parameters namedetagand two namedmatch_condition.The slot-picker in
update_clientalready chose one ifMatch/ifNoneMatch slot per operation, but it did nothing to prevent the per-parameter rename on the others.Fix
In
preprocess/__init__.pyupdate_client:ifMatchandifNoneMatchcandidates per operation, not just the first._pick_etag_slothelper prefers the standardIf-Match/If-None-Matchwire names over custom etag headers (matches pre-PR-10494 behaviour when both are present).etagRolefrom non-selected candidates soupdate_parameterleaves their naturalclientNameintact (e.g.source_if_match,source_if_none_match).Result
For
copyFromUrl:If-Match/If-None-Match→ promoted to theetag/match_conditionpair (unchanged behaviour vs. pre-PR-10494).x-ms-source-if-match/x-ms-source-if-none-match→ keep their naturalsource_if_match/source_if_none_matchclient names.No more clientName collisions.
Tests
Adds
tests/unit/test_preprocess_etag.pywith six tests:test_etag_role_preserved_when_only_standard_pair_presenttest_etag_role_preserved_when_only_custom_pair_presenttest_standard_etag_wins_over_custom_when_both_present(regression)test_first_custom_pair_chosen_when_multiple_custom_pairs_presenttest_synthetic_partner_still_works_with_only_one_custom_etagtest_full_update_yaml_does_not_collide_client_names(end-to-end)Verified the three multi-etag tests fail on
upstream/mainand pass with this change. All existing unit tests still pass.Spec referenced
The original repro is
copyFromUrlinspecification/storage/Microsoft.BlobStorage/routes.tspofAzure/azure-rest-api-specs, which composesSourceIfMatchParameter,SourceIfNoneMatchParameter,IfMatchParameter, andIfNoneMatchParameter(allAzure.Core.eTag).