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

Pin jupyter-notebook version #469

Merged
merged 2 commits into from
Jun 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 19 additions & 14 deletions stack/base/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -26,30 +26,35 @@ RUN if [ "$TARGETARCH" = "arm64" ]; then \
WORKDIR /opt/

ARG AIIDA_VERSION
# Pin shared requirements in the base environment.
# We pin aiida-core to the exact installed version,
# to prevent accidental upgrade or downgrade, that might

# Pin certain shared requirements in the base environment
# so that user cannot change their version and accidently break themselves.
# Pin aiida-core to the exact installed version
# to prevent accidental upgrade or downgrade that might
# induce DB migration or break shared dependencies of AiiDAlab Apps.
RUN echo "aiida-core==${AIIDA_VERSION}" > /opt/requirements.txt

# Install aiida-core and other shared requirements.
RUN mamba install --yes \
mamba-bash-completion \
--file /opt/requirements.txt \
&& mamba clean --all -f -y && \
fix-permissions "${CONDA_DIR}" && \
fix-permissions "/home/${NB_USER}"
# Pin jupyter-notebook to prevent downgrades or upgrades to v7
RUN echo "notebook==$(jupyter-notebook --version)" >> /opt/requirements.txt

# Pin shared requirements in the conda base environment.
RUN cat /opt/requirements.txt | xargs -I{} conda config --system --add pinned_packages {}
danielhollas marked this conversation as resolved.
Show resolved Hide resolved

# Upgrade pip to latest
RUN pip install --upgrade --no-cache-dir pip
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pip is originally installed via conda so I've changed this and use mamba update pip instead.

Copy link
Contributor Author

@danielhollas danielhollas Jun 11, 2024

Choose a reason for hiding this comment

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

Perhaps we should think about updating conda and mamba as well? (for another PR)

Copy link
Member

Choose a reason for hiding this comment

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

The mamba is from jupyter image, so I think it is safe to use any version it used, and if I remember correctly they spin up the build process periodically and will try to bring the latest mamba.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, but we're not using the latest jupyter image. In fact our current one is quite old. And even when we upgrade, I don't think we'll ever want to use the most recent one, since it would likely produce a lot of churn.

Copy link
Member

Choose a reason for hiding this comment

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

I didn't use aiidalab mamba for quite a while, did you see some annoying warnings ask to update? If so I am totally fine with updating to the latest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did not, I just think it is a good practice to keep it updated. I'll take a closer look and prepare a PR at some point.

btw: Thanks for the review, I've added some tests in case you want to take a look. Waiting for the build to finish and will then merge.

# Configure pip to use requirements file as constraints file.
# Configure pip to use the same requirements file as constraints file.
ENV PIP_CONSTRAINT /opt/requirements.txt
# Ensure that pip installs packages to ~/.local by default
ENV PIP_USER 1

# Upgrade pip to latest
RUN mamba update -y pip && mamba clean --all -f -y

# Install aiida-core and other shared requirements.
RUN mamba install --yes \
aiida-core==${AIIDA_VERSION} \
mamba-bash-completion \
&& mamba clean --all -f -y && \
fix-permissions "${CONDA_DIR}" && \
fix-permissions "/home/${NB_USER}"

# Enable verdi autocompletion.
RUN mkdir -p "${CONDA_DIR}/etc/conda/activate.d" && \
echo 'eval "$(_VERDI_COMPLETE=bash_source verdi)"' >> "${CONDA_DIR}/etc/conda/activate.d/activate_aiida_autocompletion.sh" && \
Expand Down
22 changes: 22 additions & 0 deletions tests/test_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,34 @@ def test_prevent_installation_of_aiida(
)


@pytest.mark.parametrize("pkg_manager", ["pip", "mamba"])
def test_prevent_notebook_upgrade(aiidalab_exec, nb_user, pkg_manager):
"""jupyter-notebook is pinned to the exact version in the container,
test that both pip and mamba refuse to update to v7 of the notebook."""

incompatible_version = "7"
with pytest.raises(Exception):
aiidalab_exec(
f"{pkg_manager} install notebook=={incompatible_version}",
user=nb_user,
)


def test_python_version(aiidalab_exec, python_version):
info = json.loads(aiidalab_exec("mamba list --json --full-name python"))[0]
assert info["name"] == "python"
assert parse(info["version"]) == parse(python_version)


def test_pip_version(aiidalab_exec):
"""We update pip to latest version when building the image,
test that we're not using and old pip version"""

info = json.loads(aiidalab_exec("mamba list --json --full-name pip"))[0]
assert info["name"] == "pip"
assert parse(info["version"]) >= parse("24.0")


def test_create_conda_environment(aiidalab_exec, nb_user):
output = aiidalab_exec("conda create -y -n tmp", user=nb_user).strip()
assert "conda activate tmp" in output
Expand Down