8000 MNT: Check if running inside repo2docker more explicitly by yuvipanda · Pull Request #30835 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]

Skip to content

MNT: Check if running inside repo2docker more explicitly #30835

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

Merged
merged 6 commits into from
Feb 17, 2025

Conversation

yuvipanda
Copy link
Contributor

.dockerenv is only supported in the old docker based builder, which has been deprecated for a few years now. It isn't supported by buildkit / docker buildx, which repo2docker just moved to (jupyterhub/repo2docker#1402). Using buikdkit gives us faster and more reliable builds.

This converts the test for checking if the script is being run by repo2docker to check for an env var that repo2docker sets instead.

.dockerenv is only supported in the old docker based builder, which has been deprecated for a few years now. It isn't supported by buildkit / docker buildx, which repo2docker *just* moved to (jupyterhub/repo2docker#1402). Using buikdkit gives us faster and more reliable builds.

This converts the test for checking if the script is being run by repo2docker to check for an env var that repo2docker sets instead.
@yuvipanda
Copy link
Contributor Author

I missed your comment earlier, @lesteve! Responded in #30697 (comment) :)

Copy link
github-actions bot commented Feb 14, 2025

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: f1161ce. Link to the linter CI: here

yuvipanda added a commit to yuvipanda/mybinder.org-deploy that referenced this pull request Feb 14, 2025
Brings in:

repo2docker (among others):
- jupyterhub/repo2docker#1402
- jupyterhub/repo2docker#1413

binderhub:
- jupyterhub/binderhub#1929
- jupyterhub/binderhub#1930

Note that this *could* cause some issues for the very few repos
that depend on internal details of the non buildx old builder.
For example, see scikit-learn/scikit-learn#30835
yuvipanda added a commit to yuvipanda/mybinder.org-deploy that referenced this pull request Feb 14, 2025
Brings in:

repo2docker (among others):
- jupyterhub/repo2docker#1402
- jupyterhub/repo2docker#1413

binderhub:
- jupyterhub/binderhub#1929
- jupyterhub/binderhub#1930

Note that this *could* cause some issues for the very few repos
that depend on internal details of the non buildx old builder.
For example, see scikit-learn/scikit-learn#30835

We also remove memory limits, as those are not supported right now
via the docker buildx build. We will need to add support for custom
buildkit builders in binderhub, and then we can use those.
@yuvipanda
Copy link
Contributor Author

This will break scikit-learn on binder once jupyterhub/mybinder.org-deploy#3225 lands, which is very soon.

Also note i'm using scikit-learn as one of the repos i smoke test before significant mybinder deploys :)

@yuvipanda
Copy link
Contributor Author

The change has been deployed, and this PR is now needed for the mybinder link to work.

@lesteve
Copy link
Member
lesteve commented Feb 17, 2025

Thanks for the PR and glad to know that scikit-learn is amongst your smoke test repo 🙏!

I am trying to kick off a mybinder build using this URL: https://mybinder.org/v2/gh/yuvipanda/scikit-learn/patch-2

and I am getting an error (maybe this is already known?):

Error during build: Command '['docker', 'buildx', 'build', '--progress', 'plain', '--load', '--build-arg', 'NB_USER=jovyan', '--build-arg', 'NB_UID=1000', '--tag', 'registry.2i2c.mybinder.org/i-lesteve-2dscikit-2dlearn-43aac2:3d58ad3ceb36c27d795bdb02a9fd9146d9cee3da', '--platform', 'linux/amd64', '/tmp/tmpr85dot7m']' returned non-zero exit status 1.
Waiting for build to start...
Picked Git content provider.
Cloning into '/tmp/repo2docker4tbnstyf'...
HEAD is now at 3d58ad3ce Tweak comment
Building conda environment for python=3.9
Using PythonBuildPack builder
#0 building with "default" instance using docker driver

#1 [internal] load build definition from Dockerfile
#1 transferring dockerfile: 5.24kB done
#1 DONE 0.0s

#2 [internal] load metadata for docker.io/library/buildpack-deps:jammy
#2 DONE 0.4s

#3 [internal] load .dockerignore
#3 transferring context: 2B done
#3 DONE 0.0s

#4 [ 1/20] FROM docker.io/library/buildpack-deps:jammy@sha256:1a63cc50ec6f4f45440121af59c47f78ca390607714ac0886c1588d118526b40
#4 resolve docker.io/library/buildpack-deps:jammy@sha256:1a63cc50ec6f4f45440121af59c47f78ca390607714ac0886c1588d118526b40 0.0s done
#4 DONE 0.0s

#5 [internal] load build context
#5 transferring context: 220.47MB 1.3s done
#5 DONE 1.3s

#6 [ 9/20] RUN TIMEFORMAT='time: %3R' bash -c 'time /tmp/install-base-env.bash' && rm -rf /tmp/install-base-env.bash /tmp/env
#6 CACHED

#7 [ 5/20] RUN apt-get -qq update &&     apt-get -qq install --yes --no-install-recommends        gettext-base        less        unzip        > /dev/null &&     apt-get -qq purge &&     apt-get -qq clean &&     rm -rf /var/lib/apt/lists/*
#7 CACHED

#8 [ 6/20] COPY --chown=1000:1000 build_script_files/-2fopt-2fvenv-2flib-2fpython3-2e12-2fsite-2dpackages-2frepo2docker-2fbuildpacks-2fconda-2factivate-2dconda-2esh-e67d51 /etc/profile.d/activate-conda.sh
#8 CACHED

#9 [13/20] RUN chown jovyan:jovyan /home/jovyan
#9 CACHED

#10 [ 4/20] RUN groupadd         --gid 1000         jovyan &&     useradd         --comment "Default user"         --create-home         --gid 1000         --no-log-init         --shell /bin/bash         --uid 1000         jovyan
#10 CACHED

#11 [ 8/20] COPY --chown=1000:1000 build_script_files/-2fopt-2fvenv-2flib-2fpython3-2e12-2fsite-2dpackages-2frepo2docker-2fbuildpacks-2fconda-2finstall-2dbase-2denv-2ebash-637204 /tmp/install-base-env.bash
#11 CACHED

#12 [10/20] RUN mkdir -p /srv/npm && chown -R jovyan:jovyan /srv/npm
#12 CACHED

#13 [14/20] COPY --chown=1000:1000 src/.binder/requirements.txt /home/jovyan/.binder/requirements.txt
#13 CACHED

#14 [ 7/20] COPY --chown=1000:1000 build_script_files/-2fopt-2fvenv-2flib-2fpython3-2e12-2fsite-2dpackages-2frepo2docker-2fbuildpacks-2fconda-2fenvironment-2epy-2d3-2e9-2dlinux-2d64-2elock-2e1b97 /tmp/env/environment.lock
#14 CACHED

#15 [ 3/20] RUN echo "en_US.UTF-8 UTF-8" > /etc/locale.gen &&     locale-gen
#15 CACHED

#16 [ 2/20] RUN apt-get -qq update &&     apt-get -qq install --yes --no-install-recommends locales > /dev/null &&     apt-get -qq purge &&     apt-get -qq clean &&     rm -rf /var/lib/apt/lists/*
#16 CACHED

#17 [11/20] RUN if [ ! -d "/home/jovyan" ]; then         /usr/bin/install -o jovyan -g jovyan -d "/home/jovyan";     fi
#17 CACHED

#18 [12/20] WORKDIR /home/jovyan
#18 CACHED

#19 [15/20] RUN /srv/conda/envs/notebook/bin/pip install --no-cache-dir -r ".binder/requirements.txt"
#19 CACHED

#20 [16/20] COPY --chown=1000:1000 src/ /home/jovyan/
#20 ERROR: failed to prepare rz4k8nzcrv3bwmnqjqmg42ojq as rt506a4kg25w3rc5b4ond516j: open /var/lib/docker/overlay2/mnvq1wfkrdhm3r6q5f9svdu3q/.tmp-committed2750356889: no such file or directory
------
 > [16/20] COPY --chown=1000:1000 src/ /home/jovyan/:
------
Dockerfile:124
--------------------
 122 |     
 123 |     # Copy stuff.
 124 | >>> COPY --chown=1000:1000 src/ ${REPO_DIR}/
 125 |     
 126 |     # Run assemble scripts! These will actually turn the specification
--------------------
ERROR: failed to solve: failed to prepare rz4k8nzcrv3bwmnqjqmg42ojq as rt506a4kg25w3rc5b4ond516j: open /var/lib/docker/overlay2/mnvq1wfkrdhm3r6q5f9svdu3q/.tmp-committed2750356889: no such file or directory
Error during build: Command '['docker', 'buildx', 'build', '--progress', 'plain', '--load', '--build-arg', 'NB_USER=jovyan', '--build-arg', 'NB_UID=1000', '--tag', 'registry.2i2c.mybinder.org/i-lesteve-2dscikit-2dlearn-43aac2:3d58ad3ceb36c27d795bdb02a9fd9146d9cee3da', '--platform', 'linux/amd64', '/tmp/tmpr85dot7m']' returned non-zero exit status 1.

@lesteve
Copy link
Member
lesteve commented Feb 17, 2025

It seems like the mybinder error has already been reported in Zulip.

@lesteve
Copy link
Member
lesteve commented Feb 17, 2025

Side-comment: CI failure is unrelated to this PR, see #30761. This will likely go away with another push.

@lesteve
Copy link
Member
lesteve commented Feb 17, 2025

I set auto-merge, so this will be merged automatically when CI is green!

Thanks @yuvipanda for the PR!

@lesteve lesteve enabled auto-merge (squash) February 17, 2025 09:52
@lesteve lesteve merged commit 033a46c into scikit-learn:main Feb 17, 2025
31 checks passed
@yuvipanda
Copy link
Contributor Author

ty @lesteve :) Yeah, as we bump back to doing more work on mybinder.org, i think there'll be momentary instability. However, this is from my perspective a different kind of instability - caused by us actually moving, getting stronger and more resilient than from not :) Is fixed now, and i see scikit-learn launches fine.

lesteve added a commit to lesteve/scikit-learn that referenced this pull request Feb 19, 2025
…n#30835)

Co-authored-by: Loïc Estève <loic.esteve@ymail.com>
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
0