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.

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

Jump to bottom

Use Python 3.7 everywhere #884

Merged
merged 2 commits into from Apr 10, 2019
Merged

Use Python 3.7 everywhere #884

merged 2 commits into from Apr 10, 2019

Conversation

rstorey
Copy link
Member

@rstorey rstorey commented Apr 4, 2019

No description provided.

acdha
acdha requested changes Apr 4, 2019
View reviewed changes
Copy link
Member

@acdha acdha left a comment

Choose a reason for hiding this comment

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

We should use update-alternatives instead so we don't need to touch every file which invokes Python: https://jcutrer.com/linux/upgrade-python37-ubuntu1810

@coveralls
Copy link

coveralls commented Apr 4, 2019 •
edited

Coverage Status

Coverage remained the same at 69.55% when pulling a9b963e on python-dockerfile into ede22e0 on master.

@rstorey rstorey requested a review from acdha April 8, 2019 21:08
acdha
acdha reviewed Apr 9, 2019
View reviewed changes
Copy link
Member

@acdha acdha left a comment

Choose a reason for hiding this comment

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

It doesn't look like any of these changes are necessary? The defaults for pip, python, etc. are all Python 3.7 on the python:3.7 Docker image and the rest of the changes are from #812.

entrypoint.sh Outdated
./manage.py collectstatic --clear --noinput -v0

echo Running Django dev server
echo "Running Django ASGI server"
Copy link
Member

Choose a reason for hiding this comment

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

This looks like it started from the activity-ui-prototype branch?

@rstorey
Copy link
Member Author

rstorey commented Apr 9, 2019

The Dockerfile changes for importer and celerybeat are definitely necessary.

@acdha
Copy link
Member

acdha commented Apr 9, 2019

Can we rebase this branch so it's just the Dockerfile changes?

rstorey and others added 2 commits April 9, 2019 14:02
@rstorey @acdha
Use python3.7 image for all docker containers
cf5c211
@acdha
Resync Celerybeat & Importer Dockerfiles with the main app
a9b963e 
This reuses most of the base Dockerfile to take advantage of the layer
cache, which suggests we might want to make an on-build one to further
simplify matters.
@acdha
Copy link
Member

acdha commented Apr 9, 2019

I just rebased & changed the commit to be more minimal — now it only has the python:3.7 base image change and re-syncing the apt-get invocations to take full advantage of Docker's layer caching.

@rstorey rstorey merged commit faf0372 into master Apr 10, 2019
@rstorey rstorey deleted the python-dockerfile branch April 10, 2019 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acdha acdha acdha requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rstorey @coveralls @acdha