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

Add python3.10 and fix test failures on python3.10 #1628

Merged
merged 3 commits into from
Oct 21, 2021

Conversation

MVrachev
Copy link
Collaborator

Fixes #1614

Description of the changes being introduced by the pull request:

Python 3.10 is released on October 4-th 2021 and it seems
logical to add support for it as it doesn't require any major effort
from the project.

For reference read:
https://www.python.org/downloads/release/python-3100/

I tried adding support for Python3.10 in #1610.
Then, we had CI errors due to test failures: https://github.com/theupdateframework/python-tuf/pull/1610/checks?check_run_id=3861875325
The problem comes from the fact that we start a subprocess
executing simple_https_server.py, but then we fail to communicate the
message we expect from the server process to the main process actually
running the test. We expect our custom message to be the first line
printed from the server process, but instead, a deprecation warning is
printed first about the usage of ssl.wrap_socket(). Our custom message
is printed second.
As of Python 3.7 this function has been deprecated:
https://docs.python.org/3/library/ssl.html#ssl.wrap_socket and for
whatever the reason we didn't get a warning when using it before.

My fix does what is suggested in the warning and replaces the usage of
ssl.wrap_socket() by instantiating a ssl.SSLContext object and then
calling SSLContext.wrap_socket().
This removes the warning.

Please verify and check that the pull request fulfills the following
requirements
:

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

@MVrachev
Copy link
Collaborator Author

I test locally and found that when you have Python 3.10 installed locally, then both 310 and 3.10 work when defining the new Python version in the tox.ini envlist and using it later with tox -e.
I think I prefer 3.10.

@coveralls
Copy link

coveralls commented Oct 20, 2021

Pull Request Test Coverage Report for Build 1367706007

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 97.427%

Totals Coverage Status
Change from base Build 1363818148: 0.1%
Covered Lines: 3943
Relevant Lines: 4027

💛 - Coveralls

tests/simple_https_server.py Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
@@ -15,7 +15,7 @@ jobs:
# Run regular TUF tests on each OS/Python combination, plus special tests
# (sslib master) and linters on Linux/Python3.x only.
matrix:
python-version: [3.6, 3.7, 3.8, 3.9]
python-version: ["3.6", "3.7", "3.8", "3.9", "3.10"]
Copy link
Member

Choose a reason for hiding this comment

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

Format with untyped values bites developer, part 9000 😭

I wonder how many people will independently fix this exact issue in the next year

This was referenced Oct 21, 2021
When I tried adding support for Python3.10 we had CI errors due to
test failures: https://github.com/theupdateframework/python-tuf/pull/1610/checks?check_run_id=3861875325
The problem comes from the fact that we start a subprocess
executing simple_https_server.py, but then we fail to communicate the
message we expect from the server process to the main process actually
running the test. We expect our custom message to be the first line
printed from the server process, but instead, a deprecation warning is
printed first about the usage of ssl.wrap_socket(). Our custom message
is printed second.
As of Python 3.7 this function has been deprecated:
https://docs.python.org/3/library/ssl.html#ssl.wrap_socket and for
whatever the reason we didn't get a warning when using it before.

My fix does what is suggested in the warning and replaces the usage of
ssl.wrap_socket() by instantiating a ssl.SSLContext object and then
calling SSLContext.wrap_socket().
This removes the warning.

Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Python 3.10 is released on October 4-th 2021 and it seems
logical to add support for it as it doesn't require any major effort
from the project.

For reference read:
https://www.python.org/downloads/release/python-3100/

Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
Fix GitHub workflow failures by using quotes for python versions.
It seems that adding `3.10` as a number is transformed then to `3.1`
which as a result is translated to Python version 3.1 instead of Python
version 3.10.
This seems to work for other projects as well:
https://github.com/MasoniteFramework/masonite4/blob/master/.github/workflows/pythontest.yml
https://github.com/python-pillow/Pillow/blob/main/.github/workflows/test-windows.yml
https://github.com/PyGithub/PyGithub/blob/master/.github/workflows/ci.yml

Signed-off-by: Martin Vrachev <mvrachev@vmware.com>
@MVrachev
Copy link
Collaborator Author

@jku addressed your comments.

@MVrachev MVrachev requested a review from jku October 21, 2021 11:34
Copy link
Member

@jku jku left a comment

Choose a reason for hiding this comment

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

Thank you

@jku jku merged commit 69eb29f into theupdateframework:develop Oct 21, 2021
@MVrachev MVrachev deleted the add-python3.10 branch October 27, 2021 13:55
@lukpueh lukpueh mentioned this pull request Dec 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test utils fail on every OS when using Python version 3.10
3 participants