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

Ia 4362 python upgrade #434

Merged
merged 27 commits into from
Jun 30, 2023
Merged

Ia 4362 python upgrade #434

merged 27 commits into from
Jun 30, 2023

Conversation

lucymcnatt
Copy link
Contributor

@lucymcnatt lucymcnatt commented Jun 23, 2023

Jira ticket: https://broadworkbench.atlassian.net/browse/IA-4364
Jira ticket: https://broadworkbench.atlassian.net/browse/IA-3994
Jira ticket: https://broadworkbench.atlassian.net/browse/IA-4271
Jira ticket: https://broadworkbench.atlassian.net/browse/IA-4304

Summary of changes

What

  • Updates the base image from tf-gpu.2-7 to tf-gpu.2-11.py310 to use python 3.10
  • Removes references to gatk-otvf image
  • Enables toc2 jupyter extension to allow file downloads in different formats

Why

  • Python 3.7 is EOL (no more bug patches) at the end of June
  • Intel decided to stop development of OVTF and will no longer be maintained
  • Users want to save their notebooks as HTML or other formats

@lucymcnatt lucymcnatt marked this pull request as ready for review June 29, 2023 13:33
@@ -48,7 +45,7 @@ ENV PYTHONPATH $PYTHONPATH:/usr/lib/spark/python

ENV PYSPARK_PYTHON=python3

ENV HAIL_VERSION=0.2.107
ENV HAIL_VERSION=0.2.109
Copy link
Contributor Author

Choose a reason for hiding this comment

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

keeping this up to date with the hail dockerfile

@@ -264,8 +261,6 @@ RUN curl -L http://www.repeatmasker.org/RepeatMasker/RepeatMasker-4.1.4.tar.gz \
RUN rm -f /usr/local/bin/workspace_cromwell.py
# alias cromshell_beta to cromshell
RUN echo "alias cromshell='cromshell-beta'" >> ~/.bash_aliases
# Downgrade jupyter_contrib_nbextensions as the newer version breaks snippet_menu extension
RUN pip install --force-reinstall -v "jupyter_contrib_nbextensions==0.5.1"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

downgrading this broke nbconvert

Copy link
Collaborator

Choose a reason for hiding this comment

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

You are saying that there is no need to downgrade anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as far as I know? It built fine, but I was going to maybe ask Yonghao about how to test that this extension is working when testing

# verify cromshell 2.0 is installed
RUN cromshell-beta version
# to allow file-saving in different formats
RUN jupyter nbextension enable toc2/main
Copy link
Contributor Author

Choose a reason for hiding this comment

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

USER root

COPY scripts $JUPYTER_HOME/scripts

# Add env vars to identify binary package installation
ENV TERRA_R_PLATFORM="terra-jupyter-r"
ENV TERRA_R_PLATFORM="terra-jupyter-r-1.1.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed env var to include version to resolve issue with binary package installation, thread here

Copy link
Collaborator

@LizBaldo LizBaldo left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks a bunch for the huge amount of work and testing! I mostly have a few request to document package changes in the code, and trying to rerun the flaky failed test


It is not advised to run build.sh locally, as this will push to the remote docker repo and delete the image locally upon completion.
If you're on an M1 and building an image from a locally built image, replace the current FROM command:
Copy link
Collaborator

Choose a reason for hiding this comment

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

😻

@@ -103,7 +109,7 @@ Running locally is conventient for quick development and exploring the image. Ho
- there are no environment variables like `GOOGLE_PROJECT`, `WORKSPACE_NAME`, `WORKSPACE_BUCKET`, etc when running locally
- there is no workspace-syncing when run locally

To launch an image through Terra, navigate to https://app.terra.bio, select a workspace, enter your image in the "Custom Image" field, and click Create.
To launch an image through Terra, navigate to https://app.terra.bio or your BEE's UI, select a workspace, enter your new image in the "Custom Image" field, and click Create.
Copy link
Collaborator

Choose a reason for hiding this comment

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

So happy this works!

config/conf.json Show resolved Hide resolved
@@ -1,11 +1,20 @@
[
{
"updated": "2023-02-09",
"image": "us.gcr.io/broad-dsp-gcr-public/terra-jupyter-bioconductor:2.1.9",
"updated": "2023-06-23",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ohh I think I might have forgotten to update these last time, but it is a legacy_static file so do we need to do this? Regardless it should be 2.1.11 for the bioconductor image I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh yes definitely 2.1.11! and updating these to the most recent version before the python 3.10 release was something Alessandro suggested so that users can select those if they still want 3.7. Also in the CONTRIBUTING.md it says:

The version of these legacy images is the version prior to a major or minor version bump

@@ -264,8 +261,6 @@ RUN curl -L http://www.repeatmasker.org/RepeatMasker/RepeatMasker-4.1.4.tar.gz \
RUN rm -f /usr/local/bin/workspace_cromwell.py
# alias cromshell_beta to cromshell
RUN echo "alias cromshell='cromshell-beta'" >> ~/.bash_aliases
# Downgrade jupyter_contrib_nbextensions as the newer version breaks snippet_menu extension
RUN pip install --force-reinstall -v "jupyter_contrib_nbextensions==0.5.1"
Copy link
Collaborator

Choose a reason for hiding this comment

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

You are saying that there is no need to downgrade anymore?

terra-jupyter-base/Dockerfile Show resolved Hide resolved
terra-jupyter-base/Dockerfile Outdated Show resolved Hide resolved
terra-jupyter-base/Dockerfile Show resolved Hide resolved
terra-jupyter-base/Dockerfile Show resolved Hide resolved
@lucymcnatt lucymcnatt merged commit aeedecb into master Jun 30, 2023
@lucymcnatt lucymcnatt deleted the IA-4362-python-upgrade branch June 30, 2023 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants