-
Notifications
You must be signed in to change notification settings - Fork 644
Add new scripts for creating Docker containers to enable better testing of TFQ builds #952
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
base: master
Are you sure you want to change the base?
Conversation
Installing those can hide the problem that these are not available in newer Python versions, giving a false sense of security to a developer.
Changes: * Add support for a verbose flag and a help flag * Use a better way to list the images built by this script
For some reason, the previous approach of apt-installing software-properties-common and then doing `add-apt-repository -y ppa:deadsnakes/ppa` started failing for me locally. I suspect some new security measure on Google dev systems. Switching to an approach of getting the deadsnakes list directly works better.
MichaelBroughton
left a comment
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.
Looks good. Could probably shorten things a bit with all the env vars etc, only major concern is what's happening with the install of python3-pip and the interaction with update-alternatives there.
| RUN apt-get -q update -q && \ | ||
| apt-get install -yq --no-install-recommends make clang g++ zlib1g-dev \ | ||
| python${PYTHON_VERSION} python${PYTHON_VERSION}-dev \ | ||
| python${PYTHON_VERSION}-venv python3-pip python-is-python3 |
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.
Doesn't python3-pip target the OS default python version. If we are in the process of changing it, I think this could get confused. Might want to instead use a versioned path.
| ENV TZ=UTC | ||
| ENV LANG=C.UTF-8 | ||
| ENV LC_ALL=C.UTF-8 |
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.
Nit: do we really need these ?
This adds a new subdirectory
release/docker/that contains aDockerfileand a script to create Docker images. These are useful for testing TensorFlow Quantum builds in more isolated environments with different versions of Python. The setup is not fancy, but it is useful when (e.g.) trying different combinations of dependencies with different Python versions.Running the script
create_docker_images.shin that directory creates a total of eight images, with names likeubuntu22-cp39,ubuntu22-cp310, etc. The images are based on Ubuntu Linux 22.04 and 24.04, with the addition of Python preinstalled and very little else.More information can be found in the README file
release/docker/README.md.