Skip to content

customer opt-in for ascii expansion - #48914

Open
dibahlfi wants to merge 13 commits into
mainfrom
users/dibahl/ascii-expansion-fix
Open

customer opt-in for ascii expansion#48914
dibahlfi wants to merge 13 commits into
mainfrom
users/dibahl/ascii-expansion-fix

Conversation

@dibahlfi

@dibahlfi dibahlfi commented Sep 5, 2026

Copy link
Copy Markdown
Member

Problem-
The Python SDK currently serializes JSON request bodies with ASCII escaping enabled. As a result, non-ASCII characters are expanded before being sent to Cosmos DB.

For example:

日 → \u65e5

A character that requires 3 bytes in UTF-8 becomes 6 bytes after escaping. The service measures the 2 MiB limit against the UTF-8 length of the JSON representation it receives, so documents containing large amounts of CJK or other non-ASCII text can exceed that limit even when their compact UTF-8 representation is well below it.

This blocked customers from ingesting otherwise valid documents.

Solution-
This change adds the following opt-in client option:

CosmosClient(
endpoint,
credential,
enable_compact_utf8_item_writes=True,
)

When enabled, item bodies for create, upsert, replace, patch, and transactional batch operations are serialized using compact UTF-8 instead of ASCII escape sequences.
The default remains  False , so existing applications retain their current serialization behavior.

The implementation also:

  • When the option is left at its default, item-body serialization remains byte-for-byte identical to the previous behavior. The disabled path uses the same  json.dumps(data, separators=(",", ":"))  call and  Content-Length  calculation as before. Independently, all PATCH requests now explicitly send the registered  application/json-patch+json  content type..
  • Keeps the partition-key and other semantic request headers unchanged. PATCH requests now explicitly send  Content-Type: application/json-patch+json  unless the caller supplies a content type.
  • Leaves queries, control-plane requests, scripts, responses, and pre-serialized strings unchanged.
  • Escapes surrogate code units that cannot be represented in valid UTF-8 while leaving valid Unicode characters compact, and performs that rewrite only when a document actually contains surrogate code units.
  • Preserves the existing UTF-8 Content-Length behavior, reusing the byte length produced while validating the compact body rather than encoding the full body a second time.
  • Is configured at the client level only; there is no per-request override.

Customer impact-
Customers can opt in to send large Unicode-heavy documents without unnecessary \uXXXX expansion. A document whose escaped JSON exceeds the 2 MiB request size limit may remain below it when sent as compact UTF-8.

The stored item and the values returned on reads are identical either way, since both encodings describe the same JSON document; only the bytes on the wire differ.

Validation-
Added sync and async coverage for:

  • Default and opt-in serialization.
  • Create, upsert, replace, patch, and transactional batch operations.
  • Correct UTF-8 Content-Length, including reuse of the length computed during validation.
  • Surrogate code units in keys and values.
  • Unchanged query, control-plane, header, and pre-serialized-string behavior.
  • Large CJK documents that fail with HTTP 413 when escaped but succeed with compact UTF-8.
  • Strict client-option validation and sync/async option wiring.

Copilot AI balanced review requested due to automatic review settings September 5, 2026 22:23
@dibahlfi
dibahlfi requested a review from a team as a code owner September 5, 2026 22:23
@github-actions github-actions Bot added the Cosmos label Sep 5, 2026
@dibahlfi

dibahlfi commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

@sdkReviewAgent-2

@dibahlfi

dibahlfi commented Sep 5, 2026

Copy link
Copy Markdown
Member Author

/azp run python - cosmos - tests

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
8 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Note

This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.

@dibahlfi

dibahlfi commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

@sdkReviewAgent-2

@github-actions

This comment has been minimized.

Copilot AI review requested due to automatic review settings September 6, 2026 02:05
@dibahlfi

dibahlfi commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

@sdkReviewAgent-2

@dibahlfi

dibahlfi commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

/azp run python - cosmos - tests

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@dibahlfi

dibahlfi commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

/azp run python - cosmos - tests

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Note

This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.

Copilot AI review requested due to automatic review settings September 6, 2026 03:08
@dibahlfi

dibahlfi commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

/azp run python - cosmos - tests

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@dibahlfi

dibahlfi commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

@sdkReviewAgent-2

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Note

This error may be related to your runner configuration. You can now configure runners for Copilot code review separately from Copilot cloud agent by creating a copilot-code-review.yml file with your setup steps. Read the docs for details.

Copilot AI review requested due to automatic review settings September 6, 2026 04:05
@dibahlfi

dibahlfi commented Sep 6, 2026

Copy link
Copy Markdown
Member Author

/azp run python - cosmos - tests

Copilot AI review requested due to automatic review settings September 9, 2026 00:00
@dibahlfi

dibahlfi commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

/azp run python - cosmos - tests

@dibahlfi

dibahlfi commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

@sdkReviewAgent-2

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The production wire format and request-body type change across both transport stacks warrant final human and API review despite strong coverage.

Review details
  • Files reviewed: 26/26 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Comment thread sdk/cosmos/azure-cosmos/CHANGELOG.md
Comment thread sdk/cosmos/azure-cosmos/tests/test_transport_body_encoding.py Outdated
Comment thread sdk/cosmos/azure-cosmos/tests/test_query_hybrid_search.py
Comment thread sdk/cosmos/azure-cosmos/tests/test_compact_utf8_resilience.py Outdated
@xinlian12

Copy link
Copy Markdown
Member

Review complete (59:51)

Posted 4 inline comment(s).

Steps: ✓ context, correctness, cross-sdk, design, history, past-prs, synthesis, test-coverage

Copilot AI review requested due to automatic review settings September 9, 2026 02:37
@dibahlfi

dibahlfi commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

@sdkReviewAgent-2

@dibahlfi

dibahlfi commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

/azp run python - cosmos - tests

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The unconditional PATCH header change contradicts the stated default-path compatibility guarantee.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 28/28 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread sdk/cosmos/azure-cosmos/azure/cosmos/_base.py
Comment thread sdk/cosmos/azure-cosmos/tests/test_item_body_serialization.py Outdated
Comment thread sdk/cosmos/azure-cosmos/azure/cosmos/_synchronized_request.py Outdated
@xinlian12

Copy link
Copy Markdown
Member

Review complete (19:52)

Posted 2 inline comment(s).

Steps: ✓ context, correctness, cross-sdk, design, history, past-prs, synthesis, test-coverage

Copilot AI review requested due to automatic review settings September 9, 2026 04:24
@dibahlfi

dibahlfi commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

/azp run python - cosmos - tests

@dibahlfi

dibahlfi commented Sep 9, 2026

Copy link
Copy Markdown
Member Author

@sdkReviewAgent-2

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

Copilot AI commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

One or more custom setup steps configured for this repository failed during this Copilot code review run:

Install azsdk mcp server

Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review.

Note

You can configure setup steps for Copilot code review separately from Copilot cloud agent with a copilot-code-review.yml file. Read the docs for details.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The option is correctly scoped and validated, with comprehensive sync, async, transport, resilience, and live coverage.

Review details
  • Files reviewed: 28/28 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor
[Pilot] PR Pipeline Failure Analysis

What failed

Pipeline: python - cosmos - ci / native-test / tests — build 6802858

Two emulator tests failed on platform windows2022___EmulatorTestsPython311Standard:

  • tests.test_service_retry_policies.TestServiceRetryPolicies.test_service_request_retry_policy (whl artifact)
  • tests.test_service_retry_policies.TestServiceRetryPolicies.test_service_response_retry_policy (sdist artifact)

Both fail with the same assertion pattern: the mock's retry counter is 1 when 3 is expected — i.e. retries are not iterating across all 3 preferred read regions.

Relevant pipeline output
AssertionError: assert 1 == 3
 +  where 1 = <test_service_retry_policies.TestServiceRetryPolicies.MockExecuteServiceResponseExceptionIgnoreQuery object>.counter

    mf = self.MockExecuteServiceResponseExceptionIgnoreQuery(Exception, _retry_utility.ExecuteFunction)
    with patch.object(_retry_utility, 'ExecuteFunction', mf):
        with pytest.raises(ServiceResponseError):
            container.read_item(created_item['id'], created_item['pk'])
>   assert mf.counter == 3

Root cause

This PR's diff (sdk/cosmos/azure-cosmos/**) touches item-body serialization (_synchronized_request.py, _utils.py, _base.py, _cosmos_client_connection.py, aio/_asynchronous_request.py, aio/_cosmos_client_connection_async.py, cosmos_client.py, aio/_cosmos_client.py) plus samples, docs, and new serialization tests. It does not modify tests/test_service_retry_policies.py, _retry_utility.py, _global_endpoint_manager.py, or the location-cache/read-region logic that the failing tests exercise. There is no code path connecting the compact-UTF-8 item-write feature to service-level retry-across-region behavior, so this looks like a pre-existing/flaky emulator test unrelated to this PR's changes rather than a regression it introduced.

Recommended next steps

  • Re-run the python - cosmos - ci / native-test / tests pipelines; if test_service_retry_policies passes on retry, this confirms flakiness unrelated to this PR.
  • If failures persist, investigate test_service_retry_policies.py and _global_endpoint_manager region-selection logic directly — this is outside the scope of this PR's diff and should be tracked separately.
  • See https://aka.ms/ci-fix

Copilot detected the failing pipeline and generated the analysis above. To have it attempt a fix automatically, reply with @copilot please fix the failing pipeline on this PR.

Generated by Pipeline Analysis Next Steps · auto · 59.4 AIC · ⌖ 2.3 AIC · ⊞ 9.6K ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

7 participants