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

paramiko: added supported by python 3.9 and 3.10 #1149

Merged
merged 3 commits into from
Aug 14, 2024
Merged

Conversation

sth0
Copy link
Contributor

@sth0 sth0 commented Jul 5, 2024

simple addition of python 3.9 and 3.10
in addition fixed copy of docs files
I believe all dependencies already updated.

…ct directory.

Needs updates to   gssapi-py,  pyasn1-py,  nacl-py,  cryptography-py,  bcrypt-py before submitting PR
@sth0 sth0 added enhancement waiting for owner waiting for a reply from package owner labels Jul 5, 2024
@sth0
Copy link
Contributor Author

sth0 commented Jul 5, 2024

Have sent email to maintainer.

@nieder
Copy link
Member

nieder commented Jul 20, 2024

The TestDepends pytest-relaxed-py does not exist for py39/310 @dhomeier
Our current pytest-relaxed-py-1.1.4 fails tests because our pytest is newer than it expects (lots of deprecated function errors). There's a newer upstream v2.0.2

@nieder
Copy link
Member

nieder commented Jul 20, 2024

If paramiko can be bumped to 2.12.0, which still supports py27,34-310, then libcloud-py can also be upgraded and it removes another pycrypto user.

Copy link
Contributor

@dhomeier dhomeier left a comment

Choose a reason for hiding this comment

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

Tested 2.12.0 with python310, that seems the better way to go.

Our current pytest-relaxed-py-1.1.4 fails tests because our pytest is newer than it expects (lots of deprecated function errors). There's a newer upstream v2.0.2

I would strongly recommend against adding newer pytext-relaxed variants; it would better be removed completely as it will readily wreak havoc with virtually all other pytest setups as long as they have not specified tests as the valid test directories in their configuration – which usually no-one would do since it is already the default without that plugin. That might be acceptable for a requirement in a local tox or virtual env, but not as a system-wide installed package. I have already marked it as Conflicts in tons of other packages, but better get rid of it altogether!
I've also seen comments about it being unmaintained, though there seems to have been a bit of activity since.

In fact the single test here that explicitly uses it can be easily patched to work around it; maybe some of the tests that are not named in standard nomenclature are missed this way, but I think that's the lesser evil.
paramiko-py.patch

@@ -1,7 +1,7 @@
Info2: <<
Package: paramiko-py%type_pkg[python]
Version: 2.6.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Version: 2.6.0
Version: 2.12.0

@@ -1,7 +1,7 @@
Info2: <<
Package: paramiko-py%type_pkg[python]
Version: 2.6.0
Revision: 1
Revision: 2
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Revision: 2
Revision: 1

@@ -28,7 +28,7 @@ Distribution: <<
(%type_pkg[python] = 36 ) 10.14.5,
(%type_pkg[python] = 36 ) 10.15
<<
Type: python (2.7 3.4 3.5 3.6 3.7 3.8)
Type: python (2.7 3.4 3.5 3.6 3.7 3.8 3.9 3.10)
Source: https://files.pythonhosted.org/packages/source/p/paramiko/paramiko-%v.tar.gz
Source-Checksum: SHA256(f4b2edfa0d226b70bd4ca31ea7e389325990283da23465d572ed1f70a7583041)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Source-Checksum: SHA256(f4b2edfa0d226b70bd4ca31ea7e389325990283da23465d572ed1f70a7583041)
Source-Checksum: SHA256(376885c05c5d6aa6e1f4608aac2a6b5b0548b1add40274477324605903d9cd49)

@nieder
Copy link
Member

nieder commented Jul 28, 2024

If we can update paramiko while adding, that would be better. 2.12.0 is the last version that supports py27-py35. The 3.X series are all py36 and above.

For explicitly disabling a pytest plugin, adding -p no:relaxed to the python -m pytest ... invocation should do it.

@sth0
Copy link
Contributor Author

sth0 commented Jul 29, 2024

Pushed new version to latest and removed pytest dependency as per the suggestions. Builds in maintenance mode with no errors on Python 3.10 and OS 14.5. Don't have access to other versions at the moment.

@sth0
Copy link
Contributor Author

sth0 commented Jul 31, 2024

Have not heard back from listed maintainer in 3 weeks.

@sth0 sth0 removed the waiting for owner waiting for a reply from package owner label Jul 31, 2024
@nieder
Copy link
Member

nieder commented Aug 2, 2024

Ugh. The tests still try to import pytest-relaxed, even if disabling the plugin at the pytest command (No module named 'pytest_relaxed' errors). The following are the necessary bits to get the tests to pass if pytest-relaxed is not installed:
In PatchScript, block the import, and disable the command that uses pytest-relaxed:

perl -pi -e 's|from pytest_relaxed import raises|#$&|g' tests/test_client.py tests/test_ssh_gss.py
perl -pi -e 's|\@raises|#$&|g' tests/test_client.py

In TestScript, don't load the plugin and disable the tests that try to use pytest_relaxed

TestScript: %p/bin/pytest-%type_raw[python] -p no:relaxed -k "not(PasswordPassphraseTests)" tests -vv || exit 2

This allows all the other tests to pass irrespective of pytest-relaxed being installed or not.

@nieder
Copy link
Member

nieder commented Aug 2, 2024

To add: I just saw the paramiko-py.patch that @dhomeier attached to this ticket. That also works instead of the changes that I listed above when pytest-relaxed is absent, but using the patch causes the tests to immediately crash out if pytest-relaxed-py is installed. Disabling the plugin then helps. So we need both the patch and -p no:relaxed.

@sth0
Copy link
Contributor Author

sth0 commented Aug 11, 2024

I will merge your edit and @dhomeier attached patch script. I haven't had that problem since I had a working pytest-relaxed package that seems to work. But given the discussion above I will abandon that effort and leave that package as is. I have rebuilt paramiko after removing pytest-relaxed and it builds and tests with no errors.
There are =========== 397 passed, 20 skipped, 30 warnings in 63.02s (0:01:03) ============
Almost all the warnings are deprecated "nose tests" whatever they are. There are a couple of other warnings of missing functions 'NoneType' object has no attribute '_send_user_message' Will push changes to git momentarily.

@nieder nieder merged commit 2a1b154 into fink:master Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants