search

Azure / azure-sdk-for-python

This repository is for active development of the Azure SDK for Python. For consumers of the SDK we recommend visiting our public developer docs at https://learn.microsoft.com/python/azure/ or our versioned developer docs at https://azure.github.io/azure-sdk-for-python.
MIT License
4.64k stars 2.84k forks source link

`azure.core.settings` triggers cyclic import to get `AbstractSpan` #38723

Open zooba opened 3 days ago

zooba commented 3 days ago

At this line, azure.core.settings imports azure.core.tracing to get a type - as far as I can tell, solely for type checking purposes.

https://github.com/Azure/azure-sdk-for-python/blob/347b6cbc1fda32568357d3ebf6f9ddc3f881e034/sdk/core/azure-core/azure/core/settings.py#L47

Unfortunately, this triggers an import of azure.core.pipeline:

https://github.com/Azure/azure-sdk-for-python/blob/347b6cbc1fda32568357d3ebf6f9ddc3f881e034/sdk/core/azure-core/azure/core/tracing/_abstract_span.py#L23

Which imports azure.core.pipeline._base:

https://github.com/Azure/azure-sdk-for-python/blob/347b6cbc1fda32568357d3ebf6f9ddc3f881e034/sdk/core/azure-core/azure/core/pipeline/__init__.py#L191

Which imports azure.core.pipeline.policies:

https://github.com/Azure/azure-sdk-for-python/blob/347b6cbc1fda32568357d3ebf6f9ddc3f881e034/sdk/core/azure-core/azure/core/pipeline/_base.py#L44

Which imports azure.core.pipeline.policies._distributed_tracing:

https://github.com/Azure/azure-sdk-for-python/blob/347b6cbc1fda32568357d3ebf6f9ddc3f881e034/sdk/core/azure-core/azure/core/pipeline/policies/__init__.py#L36

Which tries to import settings again:

https://github.com/Azure/azure-sdk-for-python/blob/347b6cbc1fda32568357d3ebf6f9ddc3f881e034/sdk/core/azure-core/azure/core/pipeline/policies/_distributed_tracing.py#L40

I can't explain how this is working in test suites, though I assume it's something to do with the pkgutil hacks for namespace packages (which you neither need, nor should be using for azure.core). It's very easy to construct optimised layouts (for faster install/import times) that break the flimsy hack, and I would suggest just relying on normal Python semantics. For the few cases where you genuinely want namespace packages, omit the __init__.py or add the additional search paths very explicitly.

This particular issue is also easily fixed by moving imports that are solely for type checking under if typing.TYPE_CHECKING tests, such as the one I referenced first.

github-actions[bot] commented 3 days ago

@kashifkhan @xiangyan99

github-actions[bot] commented 3 days ago

Thank you for your feedback. Tagging and routing to the team member best able to assist.