-
Notifications
You must be signed in to change notification settings - Fork 105
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 support for using Ubuntu 16 as a base instead of Debian 8 #182
Conversation
Also, change --local to use the official gcloud tool instead of our own.
Contains various bugfixes and security patches. We switch from using the Debian-provided "python3.4" packages to building it from source, in preparation for moving to newer OS base images that don't have a prebuilt Python 3.4 interpreter available.
Not ready for review, despite what GitHub thinks :) |
Ready for review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with minor comments.
build.sh
Outdated
@@ -137,7 +149,11 @@ if [ "${local}" -eq 1 ]; then | |||
fi | |||
|
|||
# Use latest released Debian as our base image |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debian/Ubuntu
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated comment
build.sh
Outdated
@@ -137,7 +149,11 @@ if [ "${local}" -eq 1 ]; then | |||
fi | |||
|
|||
# Use latest released Debian as our base image | |||
export DEBIAN_BASE_IMAGE="gcr.io/google-appengine/debian8:latest" | |||
if [ "${os_base}" == "ubuntu16" ]; then | |||
export DEBIAN_BASE_IMAGE="gcr.io/gcp-runtimes/ubuntu_16_0_4:latest" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe change the variable name to BASE_IMAGE
as it could be either debian or ubuntu?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to OS_BASE_IMAGE
This requires building Python 3.4 from source, so we do that.
Future changes may add support for other OS base images, such as Debian 9 and Ubuntu 18. We don't support arbitrary OS base images in general, so it's an enumerated list.