Skip to content

Add support for Python3.10 on testing Docker image #8328

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Aug 2, 2021
Merged

Add support for Python3.10 on testing Docker image #8328

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5e013c5
Add support for Python3.10 on `testing` Docker image
humitos Jul 8, 2021
d9c9ed6
Comment about potential problems with float values
humitos Jul 21, 2021
7f83fcd
Allow to use 3.10 as string in YAML config file
humitos Jul 26, 2021
80b3c2c
Merge branch 'master' of github.com:readthedocs/readthedocs.org into …
humitos Jul 26, 2021
b38cd36
Merge branch 'master' into humitos/py310-testing-image
stsewd Jul 26, 2021
3222858
Config file: use string for python.version (#8372)
stsewd Aug 2, 2021
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
2 changes: 1 addition & 1 deletion readthedocs/settings/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -490,7 +490,7 @@ def TEMPLATES(self):
},
'readthedocs/build:7.0': {
'python': {
'supported_versions': [2, 2.7, 3, 3.5, 3.6, 3.7, 3.8, 3.9, 'pypy3.5'],
'supported_versions': [2, 2.7, 3, 3.5, 3.6, 3.7, 3.8, 3.9, 3.10, 'pypy3.5'],
Copy link
Member

Choose a reason for hiding this comment

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

Seems like these should probably be strings, but guessing that ship has sailed. Are they floats now?

Copy link
Member Author

@humitos humitos Jul 12, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I don't remember this setting being a different type than float. We are accepting floats and string in the config file, but if it's a string we are converting it to float. I found we have a test case for that https://github.com/readthedocs/readthedocs.org/blob/884132a5b55a6d5e7e70d8da4856f47df1d352d7/readthedocs/config/tests/test_config.py#L384-L399

So, I suppose that using string here will require more work.

... I just realized that I'm sleepy 😴 ...

This may be a problem, I guess. We will be comparing against 3.1 --which, it will work with because we are changing python: version: "3.10" to float as well ending up as 3.1, but it will eventually explode 💣

Copy link
Member Author

Choose a reason for hiding this comment

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

If we ever hit this problem, it would probably be in Python 3.50 because we already accept Python 3.5 😅

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it really too late to change this? 🙈

Copy link
Member Author

Choose a reason for hiding this comment

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

@stsewd how much work would be to change python.version to be support float and string and use the string version in all our internal code? (config file, build environment, tests, settings, etc).

Also, we should change our documentation to de-motivate the usage of float on this config and re-write all the occurrences to use the string version.

Copy link
Member

Choose a reason for hiding this comment

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

should be fine. We used to compare the versions as numbers to get the highest version for 3 and 2, but we use another method for that now.

Copy link
Member Author

Choose a reason for hiding this comment

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

As this doesn't affect us immediately, I don't want to block the upgrade to the new Docker image because of this problem. I opened this issue to track this change: #8361

Copy link
Member Author

Choose a reason for hiding this comment

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

Meh, it doesn't work when doing the build /bin/sh: 1: python3.1: not found

Copy link
Member Author

Choose a reason for hiding this comment

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

I pushed a commit to support "3.10" as string in python.version.

However, I had to made some decisions:

  • using string in python.version is not cast to int/float anymore
  • using "3.6" (as string) in the YAML keeps working for backward compatibility

This helps us to keep all the config files working as they were. However, I had to adapt our code to:

  • add all the string versions for python.supported_versions in our settings file
  • update some tests to not auto-cast python version
  • allow python.supported_versions to use int, float, and string versions
  • add a specific case for 3.10 when getting the python_interpreter

These changes are enough to make 3.10 work on our platform. I didn't find any issue with them yet, but it may be good to take a deeper look in the future and eventually remove the support for int/float values on python.version in the config file.

'default_version': {
2: 2.7,
3: 3.7,
Expand Down