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

Python repo is not built if CondaBuildPack is selected #1265

Open
TomWinder opened this issue Apr 14, 2023 · 3 comments
Open

Python repo is not built if CondaBuildPack is selected #1265

TomWinder opened this issue Apr 14, 2023 · 3 comments

Comments

@TomWinder
Copy link

Bug description

When repo2docker sees an environment.yml file, it selects the CondaBuildPack. This means it skips building the package that is contained in the repo itself.

Expected behaviour

I would expect that whether a conda environment.yml file is provided or not, there would still be a check for whether a setup.py file is present, as this indicates pip install . or equivalent is required to build the package contained in the repo.

Actual behaviour

repo2docker selects the CondaBuildPack, and the resulting image does not have the repo installed.

How to reproduce

  1. Build a repo that contains an environment.yml file, as well as a setup.py file
  2. Build with binder / repo2docker; CondaBuildPack will be selected
  3. Resulting image environment will not have the repo package installed, as setup.py was never run.
  4. (Attempt to import the repo package in a notebook --> ModuleNotFoundError)
    image

Your personal set up

  • OS: macOS 11.4 (plus whichever OS is used to build Binder images)
  • Docker version: 20.10.23 // whatever binder is using
  • repo2docker version: 2022.10.0 // whatever binder is using

See failed build logs from binder attached. Failed notebook builds (from https://github.com/QuakeMigrate/QuakeMigrate/tree/master) are here

On a separate fork ( https://github.com/TomWinder/QuakeMigrate/tree/fix_binder) I renamed environment.yml, resulting in a successful build using the PythonBuildPack. Logs are also attached, and the notebooks are here.

Note that in the CondaBuildPack env/image, I can use pip install . to rectify the issue. But I would again expect this to be done during the repo2docker build.

Apologies if there is an example of how to deal with this situation somewhere, I have searched pretty hard and not yet found anything.

See also: QuakeMigrate/QuakeMigrate#155

@welcome
Copy link

welcome bot commented Apr 14, 2023

Thank you for opening your first issue in this project! Engagement like this is essential for open source projects! 🤗

If you haven't done so already, check out Jupyter's Code of Conduct. Also, please try to follow the issue template as it helps other other community members to contribute more effectively.
welcome
You can meet the other Jovyans by joining our Discourse forum. There is also an intro thread there where you can stop by and say Hi! 👋

Welcome to the Jupyter community! 🎉

@TomWinder
Copy link
Author

Note there is a relevant Discourse post here. However, I have left this Issue open, because the proposed solutions do not work in the general case that a repo needs to be built from source.

Specifically, the suggestion to add:

 - pip:
   - .

to the environment.yml file does not seem work on the current version of binder. See full log attached, with the key error message being:

Installing pip dependencies: ...working... �[91mPip subprocess error:
ERROR: Directory '.' is not installable. Neither 'setup.py' nor 'pyproject.toml' found.

�[0mRan pip subprocess with arguments:
['/srv/conda/envs/notebook/bin/python', '-m', 'pip', 'install', '-U', '-r', '/home/jovyan/.binder/condaenv.6jdbvr34.requirements.txt', '--exists-action=b']
Pip subprocess output:

failed

TomWinder added a commit to QuakeMigrate/QuakeMigrate that referenced this issue Apr 14, 2023
fix(binder): Add separate `environment.yml` file in `.binder/`

Binder builds were no longer working after renaming the environment file from
`quakemigrate.yml` --> `environment.yml`. See jupyterhub/repo2docker#1265
for a full rundown.

As a workaround, this commit adds a separate environment file for use in building
the binder images. This doesn't interfere with the default environment file etc,
and installs `quakemigrate` to the image from PyPI. This does add another file to
update, and introduces the potential for the `master` branch to diverge from
the binder build, but these are acceptable compromises.

This PR closes #155
@bollwyvl
Copy link
Contributor

Using pip: ["-e ./path/to/package"] can work, in certain cases, but breaks the benefits of the "big old conda layer" which caches fairly well at this point (unless it also has apt.txt), as it will invalidate the layer cache on basically any change.

In this case, the (basically) one-liner ./.binder/postBuild is probably the better bet to couple with environment.yml that captures the full build environment for an efficient, observable, reproducible build:

#!/usr/bin/env bash
set -eux
python -m pip install -vv --no-deps --no-build-isolation -e path/to/package/

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

No branches or pull requests

2 participants