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

bug: unintended creation of wheel directory #3528

Closed
jiewpeng opened this issue Feb 7, 2023 · 1 comment · Fixed by #3532
Closed

bug: unintended creation of wheel directory #3528

jiewpeng opened this issue Feb 7, 2023 · 1 comment · Fixed by #3532
Labels
bug Something isn't working

Comments

@jiewpeng
Copy link
Contributor

jiewpeng commented Feb 7, 2023

Describe the bug

When the user specifies Python options with wheels as empty list instead of None, bentoml will create the wheel directory, causing install.sh to fail because it tries to install *.whl from the wheels directory, but there are no wheel files inside.

This is normally not an issue because the user is expected to create a bentofile.yaml and use the bentoml CLI to build their bentos (and the user will typically not specify the wheels section at all if there are none), but in specific workflows where it is more convenient to use the bentoml Python interface to build bentos (e.g. when the user is using a notebook-centric environment like Databricks), the user may specify an empty list for wheels instead of None.

It is fairly simple to workaround this (we can specify None instead of empty list when calling bentoml.bentos.build()), but it would be nice if it could work with empty list as well. I can work on this, just let me know if it's ok to proceed.

if self.wheels is not None:
    bento_fs.makedirs(wheels_folder, recreate=True)
    for whl_file in self.wheels:  # pylint: disable=not-an-iterable
        whl_file = resolve_user_filepath(whl_file, build_ctx)
        copy_file_to_fs_folder(whl_file, bento_fs, wheels_folder)
if [ -d "$WHEELS_DIR" ]; then
    echo "Installing wheels packaged in Bento.."
    pip3 install "$WHEELS_DIR"/*.whl "${PIP_ARGS[@]}"
fi

I believe the check should be simplified to if self.wheels, so it covers the case of empty list as well.

To reproduce

No response

Expected behavior

No response

Environment

bentoml: 1.0.8 onwards
Python: 3.8

@jiewpeng jiewpeng added the bug Something isn't working label Feb 7, 2023
@aarnphm
Copy link
Contributor

aarnphm commented Feb 7, 2023

I think this can be fixed by simply change

if self.wheels is not None: ...

to

if self.wheels: ...

should cover your case.

The reason for checking is not None is that we want to avoid type coercion in Python. But I guess it is a thing in Python

aarnphm pushed a commit that referenced this issue Feb 7, 2023
Coerce types to check if wheels exist, then create the directory.

Fixes #3528
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants