Skip to content

[azure-core] Search for charset detection library at import time#43092

Merged
annatisch merged 8 commits intoAzure:mainfrom
yuliy-openai:one_charset_import
Sep 30, 2025
Merged

[azure-core] Search for charset detection library at import time#43092
annatisch merged 8 commits intoAzure:mainfrom
yuliy-openai:one_charset_import

Conversation

@yuliy-openai
Copy link
Contributor

@yuliy-openai yuliy-openai commented Sep 23, 2025

Description

The AioHttpTransport class would try to import cchardet every time it got a request where it wanted to use charset detection on an http response. This requires syscalls to look for the module, which in the case of something which makes heavy use of, say, the Cosmos SDK, adds up to a lot of resource utilization. Doing it once is faster. I opted to keep the old behavior of erroring out only when this code is called.

This was a regression from #21520

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

Copilot AI review requested due to automatic review settings September 23, 2025 06:28
@github-actions github-actions bot added Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Sep 23, 2025
@github-actions
Copy link

Thank you for your contribution @yuliy-openai! We will review the pull request and get back to you soon.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR optimizes the charset detection library import strategy in the AioHttpTransport class by moving import attempts from request-time to module-import-time, reducing syscall overhead for applications making frequent HTTP requests.

  • Moves charset detection library imports (cchardet, chardet, charset_normalizer) from the text() method to module-level
  • Creates a fallback detect function that preserves the original error behavior when no charset libraries are available
  • Updates the CHANGELOG to document this performance fix

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
sdk/core/azure-core/azure/core/pipeline/transport/_aiohttp.py Moves charset detection imports to module level and simplifies the text() method
sdk/core/azure-core/CHANGELOG.md Documents the bug fix for repeated import attempts

@annatisch
Copy link
Member

Thanks @yuliy-openai - could you please agree to the CLA policy above?

Copy link
Member

@pvaneck pvaneck left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@yuliy-openai
Copy link
Contributor Author

could you please agree to the CLA policy above?

I'm still chasing down our legal team for that :-(

@yuliy-openai
Copy link
Contributor Author

@microsoft-github-policy-service agree company="OpenAI"

@annatisch
Copy link
Member

/check-enforcer override

@annatisch annatisch merged commit 129e858 into Azure:main Sep 30, 2025
53 checks passed
@yuliy-openai
Copy link
Contributor Author

Thanks for your help and patience, @annatisch!

@annatisch
Copy link
Member

You're welcome! Thank you for your contribution!

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

Labels

Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants