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.62k stars 2.83k forks source link

[azure-identity] `azure-identity` should stop using deprecated `backend` argument in `cryptography` #36579

Open jiasli opened 3 months ago

jiasli commented 3 months ago

Describe the bug Per

cryptography stopped requiring the use of backend arguments in version 3.1 and deprecated their use in version 36.0. If you are on an older version that requires these arguments please view the appropriate documentation version or upgrade to the latest release.

Note that for forward compatibility backend is still silently accepted by functions that previously required it, but it is ignored and no longer documented.

Expected behavior Azure Identity currently requires

https://github.com/Azure/azure-sdk-for-python/blob/d840732813529eaf8551422a1497bedeb21a1fe1/sdk/identity/azure-identity/setup.py#L63

and passes backend arguments:

https://github.com/Azure/azure-sdk-for-python/blob/07d10639d7e47f4852eaeb74aef5d569db499d6e/sdk/identity/azure-identity/azure/identity/_credentials/certificate.py#L95-L96

but the best practice is to bump cryptography to newer version and stop using deprecated backend argument.

github-actions[bot] commented 3 months ago

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

xiangyan99 commented 3 months ago

To help me understand better, what is the benefit for this?

It seems to me the impact is we stop allowing users to use cryptography 2.5.

jiasli commented 3 months ago

To help me understand better, what is the benefit for this?

There is no immediate benefit, but it avoids being broken by cryptography when backend is totally removed.

This issue is merely a tracker. There is no action item currently.

rayluo commented 3 months ago

To help me understand better, what is the benefit for this?

There is no immediate benefit, but it avoids being broken by cryptography when backend is totally removed.

This issue is merely a tracker. There is no action item currently.

FWIW, I also feel @xiangyan99 's reluctance to "the impact is we stop allowing users to use cryptography 2.5".

Besides, crytography does not use semantic versioning, so, there is no obvious way for a mid-tier/downstream library to guard against a potential breaking change.

I end up with a complicated mechanism of wrapping most of MSAL's cryptography usage into a helper function, and then have a unit test case to call that helper. If that test case fails someday, we will know we hit the breaking change.