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

Support Docker 5.0 image #5657

Merged
merged 3 commits into from May 20, 2019
Merged

Support Docker 5.0 image #5657

merged 3 commits into from May 20, 2019

Conversation

@humitos
Copy link
Member

@humitos humitos commented May 2, 2019

Changes on this PR:

  • Use 4.0 as stable and 5.0 as latest for our Docker images
  • Allow to specify pypy3.5 as Python version (see #5658)
  • Adapt tests relying in Python 3.5 to use 3.6 or 3.7 since 3.5 is not supported anymore

Required by readthedocs/readthedocs-docker-images#103.

@humitos humitos force-pushed the humitos/docker-5.0-image-release branch from f811cee to faf4128 May 2, 2019
@humitos humitos force-pushed the humitos/docker-5.0-image-release branch from faf4128 to 714cfd9 May 2, 2019
@humitos humitos requested a review from May 2, 2019
stsewd
stsewd approved these changes May 2, 2019
Copy link
Member

@stsewd stsewd left a comment

Changes look good, I should test this locally.

Also, I'm a little worry about removing python 3.5, I think there were some users using it with the latest image bc of a problem with a package. Solutions could be using pypy3.5 (if it works) or downgrading the image version to stable or using a numbered version.

return 'python{}'.format(ver)

# Allow to specify ``pypy3.5`` as Python interpreter
return ver
Copy link
Member

@stsewd stsewd May 2, 2019

Just a note that we can do this better later #4343

@humitos
Copy link
Member Author

@humitos humitos commented May 2, 2019

I'm a little worry about removing python 3.5, I think there were some users using it with the latest image bc of a problem with a package

Is there some code that runs in 3.5 but does not in 3.6 or 3.7?

@stsewd
Copy link
Member

@stsewd stsewd commented May 2, 2019

I was able to find this one #5250 (comment), not sure, I can remember some weird packages. Also, we should query the db to see the python_version of projects just in case.

@humitos
Copy link
Member Author

@humitos humitos commented May 6, 2019

Marked as blocked since we need to update the builders with the new images, first.

@humitos
Copy link
Member Author

@humitos humitos commented May 15, 2019

Also, we should query the db to see the python_version of projects just in case.

It's hard to create a good query because we are using a TextField on Build._config. Although, this seems to work for this case:

In [56]: Project.objects.filter(builds___config__contains='"python":{"version":3.5,', users__profile__banned=False).distinct().count()
Out[56]: 320

In [57]: Counter([p.get_latest_build().config.get('build', {}).get('image', 'No image') for p in Project.objects.filter(builds___config__contains='"python":{"version":3.5,', users__profile__
    ...: banned=False).distinct()])
Out[57]: 
Counter({'readthedocs/build:latest': 266,
         'readthedocs/build:2.0': 47,
         'readthedocs/build:stable': 3,
         'No image': 4})

So, project using readthedocs/build:latest (or no image) and 3.5 (266 + 4) will fail on next build. None of them looks like SPAM projects (taking a look at the Project.slug) and I think none of them should be SPAM probably because they have a YAML file with a very specific config.


What should be the path to follow here?

I do see three different ones at the moment:

  1. make our config parser to increase 3.x versions to the next one available in the image selected.
    • if the user defined 3.5 but only 3.6, 3.7 are available in the image chosen, use 3.6 to build the docs
  2. re add 3.5 in 5.0 Docker image and make a deprecation plan giving some time to user to upgrade
  3. make those builds fail informing that 3.5 is not supported anymore and inviting the user to upgrade their config file
    • use only 3 as Python version to use a new version
    • force stable build image which has 3.5 (*)

(*) we will have the same problem when deploying our 6.0 image, though.

@stsewd
Copy link
Member

@stsewd stsewd commented May 15, 2019

1 would be weird and unexpected from the user perspective. I'm +1 for 2, we could do the deprecation blog post this week. Do we need to release the image right now to fix something? If not, I guess it could wait, so we don't need to re-add 3.5 there.

@humitos
Copy link
Member Author

@humitos humitos commented May 16, 2019

I just came up with another idea:

  1. Assign Project.container_image = 'readthedocs/build:4.0' to those project using Python 3.5.

That way, we can keep those project building properly but also upgrade the Docker image.

humitos added a commit to readthedocs/readthedocs-docker-images that referenced this issue May 16, 2019
Python 3.5 was removed at
#84 and we
didn't have a good reason to do it.

Then, when releasing the new Docker 5.0 image we realize that we will
be breaking some projects that are pinning 3.5 in their config file:
readthedocs/readthedocs.org#5657

So, we decided to re-add it to be able to deploy this image without
breaking people's projects.
@humitos humitos force-pushed the humitos/docker-5.0-image-release branch from 74d6fe6 to 1513a5f May 16, 2019
@stsewd
Copy link
Member

@stsewd stsewd commented May 16, 2019

Assign Project.container_image = 'readthedocs/build:4.0'

I know we are re adding 3.5. But just to note that when you set project.container_image users are stuck with that image (it overrides whatever users have in their config file)

stsewd
stsewd approved these changes May 16, 2019
@humitos
Copy link
Member Author

@humitos humitos commented May 20, 2019

Docker 5.0 image was updated with Python 3.5 and this PR was upgraded. Also, build servers already have the new image.

I'm merging this PR to go out in the next deploy.

@humitos humitos merged commit 2fb9eae into master May 20, 2019
2 checks passed
@delete-merged-branch delete-merged-branch bot deleted the humitos/docker-5.0-image-release branch May 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants