Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

{Package} remove upper bound in cryptography dependency #15104

Closed
wants to merge 1 commit into from

Conversation

bluca
Copy link
Member

@bluca bluca commented Sep 9, 2020

It does not seem necessary, none of the changes appear to affect azure-cli usage.

https://github.com/pyca/cryptography/blob/master/CHANGELOG.rst

Distributions have already moved past 3.0, so this restriction is
problematic.


This checklist is used to make sure that common guidelines for a pull request are followed.

It does not seem necessary, none of the changes appear to affect azure-cli usage.

https://github.com/pyca/cryptography/blob/master/CHANGELOG.rst

Distributions have already moved past 3.0, so this restriction is
problematic.
@yungezz
Copy link
Member

yungezz commented Sep 9, 2020

hi @fengzhou-msft @bim-msft could you pls help review the change? what's impact to keyvault?

@bim-msft
Copy link
Contributor

Hi @bluca thanks for your feedback.

@yungezz I will do a full regression test on KeyVault later to make sure everything goes well.

@yungezz yungezz added the KeyVault az keyvault label Sep 11, 2020
@bluca
Copy link
Member Author

bluca commented Oct 9, 2020

ping

@yungezz
Copy link
Member

yungezz commented Oct 9, 2020

hi @bluca sorry for late response, we're full with Ignite release before. Will test it then

@bluca
Copy link
Member Author

bluca commented Oct 9, 2020

hi @bluca sorry for late response, we're full with Ignite release before. Will test it then

Sure no problem, thanks

@yungezz yungezz assigned fengzhou-msft and unassigned bim-msft Dec 22, 2020
@@ -126,7 +126,7 @@
'azure-storage-common~=1.4',
'azure-synapse-accesscontrol~=0.2.0',
'azure-synapse-spark~=0.2.0',
'cryptography>=2.3.1,<3.0.0',
'cryptography>=2.3.1',
Copy link
Member

Choose a reason for hiding this comment

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

normal practice in CLI is pin exact major version. what's impact of the change?

Copy link
Member Author

@bluca bluca Dec 23, 2020

Choose a reason for hiding this comment

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

It might be common, but I'm afraid it's not good practice to pin without any specific reason - eg, a specific incompatibility. There is no incompatibility introduced in 3.0 here, so it's unnecessary. The impact is that python3-cryptography is moving on everywhere, and the old deprecated versions are not available anymore, so we have to manually patch this downstream, which is causing extra work.

https://packages.debian.org/search?keywords=python3-cryptography&searchon=names&suite=all&section=all
https://packages.ubuntu.com/search?keywords=python3-cryptography&searchon=names&suite=all&section=all
https://rpmfind.net/linux/rpm2html/search.php?query=python3-cryptography

@fengzhou-msft
Copy link
Member

@bluca we've bumped cryptography:

'cryptography>=3.2,<3.4',

Due to latest versions relying on rust and breaking installation on platforms like alpine we used for docker image, it's still upper bounded.

@bluca
Copy link
Member Author

bluca commented Feb 24, 2021

That's great, thank you!

@bluca bluca closed this Feb 24, 2021
@bluca bluca deleted the python_crypto_version branch February 24, 2021 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants