Readthedocs.org: Add `default_python_version` to `DOCKER_IMAGE_SETTINGS`

Created on 21 Jan 2020  路  11Comments  路  Source: readthedocs/readthedocs.org

We are currently configuring available docker images for builders at

https://github.com/readthedocs/readthedocs.org/blob/4d066309e8abd8b2fb2789c62e76f3f4b9ccf1ad/readthedocs/settings/base.py#L370-L389

At the moment, we don't have a way to pin which version of Python will be used by default on each of these images. This code says that the default Python version will be always the greatest on each image:

https://github.com/readthedocs/readthedocs.org/blob/4d066309e8abd8b2fb2789c62e76f3f4b9ccf1ad/readthedocs/config/config.py#L245-L264

We want to update the DOCKER_IMAGE_SETTINGS to have a default_python_version per image and those linked properties to use them if defined.

This allows us to release a new version of Docker image with a newer version of Python without forcing all users using latest to automatically start using the newer version of Python.

Required by: #6324

Accepted Improvement Sprintable

All 11 comments

@humitos It seems in python_full_version property we are tying to get the max for each major versions of python (2, 3). So, If we add only one default version then how are we going to differentiate between python 2 and 3. for example, if user sets 2 as the version and we have 3.6 as default.

@saadmk11 yeah, we should implement it like

'default_version': {
    2: 2.7,
    3: 3.7,
} 

@stsewd Will adding a default_version key next to supported_versions key in every build do the work? Also, which python version should be the default in every build? (the oldest one?)

Anyone is working in this issue?
I have added default version using the latest python version listed in supported versions, as @preetmishra mentioned:

'readthedocs/build:6.0rc1': {
    'python': {
        'supported_versions': [2, 2.7, 3, 3.5, 3.6, 3.7, 3.8, 'pypy3.5'],
        'default_version': {
            2: 2.7,
            3: 3.8,
        }
    },
},

This solve this issue completely?

@joaovitor3 Yes, I'd like to work on this issue, if it seems doable. I am waiting for the reply from @stsewd.

No problem, you can work on this @preetmishra
Thanks for the reply.

Will adding a default_version key next to supported_versions key in every build do the work?

No. You need to also find where this needs to be used (I linked the code in the description). Please, add some tests for this and make sure that it picks the right Python version when building in your RTD instance.

Also, which python version should be the default in every build? (the oldest one?)

See the comment from @stsewd at https://github.com/readthedocs/readthedocs.org/issues/6551#issuecomment-576778461

@humitos Thanks for reverting back. I understood how default_version should be like from the comment.
I have a few more questions.

  • Should the default python version be the same for every build or should it be different?
  • Which python version should we choose? The oldest version (say 2 for python 2 and 3 for python 3 for readthedocs/build:6.0rc1) or the newest version (say 2.7 for python 2 and 3.8 for python 3 for readthedocs/build:6.0rc1)?

@humitos sorry, but you complete container build approach is wrong...

At first, containers include software and libraries they need to fulfill their purpose. Having all Python versions in parallel in on container is wrong.

At second, (Docker) containers are build in layers. You create a base container and on top you create variations. That's like a Git commit on branch master and having multiple branches diverting from from that base commit.

  1. Build base containers for an environment supporting Pythons 2.7 and 3.3 ... 3.8
  2. Use a Python x.y container to create derived containers for RTD 1.0 ... 6.0rc1

This should result in these containers:

  • readthedocs/build-py2.7:1.0
  • readthedocs/build-py2.7:2.0
  • readthedocs/build-py2.7:3.0
  • readthedocs/build-py2.7:4.0
  • readthedocs/build-py2.7:5.0
  • readthedocs/build-py2.7:6.0rc1
  • readthedocs/build-py3.3:3.0
  • readthedocs/build-py3.4:1.0
  • readthedocs/build-py3.4:3.0
  • readthedocs/build-py3.5:2.0
  • readthedocs/build-py3.5:3.0
  • readthedocs/build-py3.5:4.0
  • readthedocs/build-py3.5:5.0
  • readthedocs/build-py3.5:6.0rc1
  • readthedocs/build-py3.6:3.0
  • readthedocs/build-py3.6:4.0
  • readthedocs/build-py3.6:5.0
  • readthedocs/build-py3.6:6.0rc1
  • readthedocs/build-py3.7:4.0
  • readthedocs/build-py3.7:5.0
  • readthedocs/build-py3.7:6.0rc1
  • readthedocs/build-py3.8:6.0rc1

If you think about resource consumption, that not a big deal, because all build-pyX.Y versions share the same container layer. Only your application on top creates overhead, because you do a matrix build. You could now review if you need a Cartesian product or if a sparse matrix is enough.

You don't run into a multi-version and default-version problem, because your image selected the appropriate Python environment.

Thanks for the tip, @Paebbels. There are other things to consider as well before changing how we handle our docker images. However, I think the effort we need for adding a config setting (current issue) is very low compared to changing all of this, and we will get the same benefit.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

gtalarico picture gtalarico  路  4Comments

krzychb picture krzychb  路  4Comments

davidfischer picture davidfischer  路  4Comments

PowerKiKi picture PowerKiKi  路  4Comments

boscorelly picture boscorelly  路  4Comments