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

remove cryptography version pinning due to paramiko newer than 2.11.0 #1109

Merged
merged 5 commits into from
Mar 24, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
Upcoming
========

Internal:
---------
* paramiko is newer than 2.11.0 now, remove version pinning `cryptography`.

1.27.0 (2023/08/11)
===================

Expand Down
1 change: 1 addition & 0 deletions mycli/AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ Contributors:
* Arvind Mishra
* Kevin Schmeichel
* Mel Dafert
* Thomas Copper

Created by:
-----------
Expand Down
5 changes: 2 additions & 3 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@

install_requirements = [
'click >= 7.0',
# Temporary to suppress paramiko Blowfish warning which breaks CI.
# Pinning cryptography should not be needed after paramiko 2.11.0.
'cryptography == 36.0.2',
# Pinning cryptography is not needed after paramiko 2.11.0. Correct it
'cryptography >= 1.0.0',
Copy link
Member

Choose a reason for hiding this comment

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

If pinning is not needed, I think we can just delete this line, because we didn't depend on cryptography directly from code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I commit this pull request 11 months ago, at that time, I checked the commit history and found that cryptography >= 1.0.0 had been there before, and the version pinning to ==36.0.2 is edited from >= 1.0.0, so I thought it is better to revert to >= 1.0.0 than directly delete it.

Now that 11 months has passed and there are a lot of new commits, if it is better to directly delete this line, I will happily agree with it.

Copy link
Member

Choose a reason for hiding this comment

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

apologies for the delay 🥺

can you resolve the conflict as well? thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

apologies for the delay 🥺

can you resolve the conflict as well? thanks

Do you mean the "Resolve conversation" button below?

Copy link
Member

Choose a reason for hiding this comment

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

No, I mean, this PR can not be merged now, cause it has conflicts.

image

Can you rebase from main branch, or resubmit one? thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I mean, this PR can not be merged now, cause it has conflicts.
image

Can you rebase from main branch, or resubmit one? thanks.

Oh,please allow me some time.

# 'Pygments>=1.6,<=2.11.1',
'Pygments>=1.6',
'prompt_toolkit>=3.0.6,<4.0.0',
Expand Down
Loading