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

feat: conda env for bentos in bentostore #3396

Merged
merged 26 commits into from
Feb 8, 2023

Conversation

jjmachan
Copy link
Contributor

@jjmachan jjmachan commented Jan 6, 2023

Env Manager

The env manager allows you to run bentoml serve in an isolated environment. This PR adds support for creating conda environment

Usage

> bentoml serve --env conda <bento>

The created environment is created under $BENTOML_HOME/env if <bento> is loaded to bentostore. For project-level invocation the created environment is ephemeral.

To clean up the environment created, rm -rf $BENTOML_HOME/env

Before submitting:

src/bentoml/_internal/env_manager/manager.py Outdated Show resolved Hide resolved
src/bentoml/_internal/env_manager/manager.py Outdated Show resolved Hide resolved
src/bentoml/_internal/env_manager/manager.py Outdated Show resolved Hide resolved
src/bentoml/_internal/env_manager/manager.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jan 20, 2023

Codecov Report

Merging #3396 (5d29c6d) into main (508d816) will decrease coverage by 0.26%.
The diff coverage is 7.01%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3396      +/-   ##
==========================================
- Coverage   31.93%   31.67%   -0.26%     
==========================================
  Files         135      138       +3     
  Lines       11150    11263     +113     
  Branches     1839     1856      +17     
==========================================
+ Hits         3561     3568       +7     
- Misses       7346     7452     +106     
  Partials      243      243              
Impacted Files Coverage Δ
src/bentoml/_internal/env_manager/__init__.py 0.00% <0.00%> (ø)
src/bentoml/_internal/env_manager/envs.py 0.00% <0.00%> (ø)
src/bentoml/_internal/env_manager/manager.py 0.00% <0.00%> (ø)
src/bentoml/bentos.py 45.90% <0.00%> (-0.77%) ⬇️
src/bentoml/_internal/configuration/containers.py 48.43% <72.72%> (+1.01%) ⬆️

Copy link
Contributor

@aarnphm aarnphm left a comment

Choose a reason for hiding this comment

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

fwiw for all newer type we should use https://peps.python.org/pep-0604/ for consistency.

There are some typo here and there. I will dm you on slack for more.

src/bentoml/_internal/env_manager/manager.py Outdated Show resolved Hide resolved
src/bentoml/_internal/env_manager/manager.py Outdated Show resolved Hide resolved
src/bentoml/_internal/env_manager/manager.py Outdated Show resolved Hide resolved
src/bentoml/_internal/env_manager/manager.py Outdated Show resolved Hide resolved
src/bentoml/_internal/env_manager/manager.py Outdated Show resolved Hide resolved
src/bentoml_cli/env_manager.py Show resolved Hide resolved
src/bentoml/_internal/env_manager/manager.py Outdated Show resolved Hide resolved
src/bentoml_cli/env_manager.py Outdated Show resolved Hide resolved
src/bentoml_cli/env_manager.py Outdated Show resolved Hide resolved
src/bentoml_cli/env_manager.py Outdated Show resolved Hide resolved
@jjmachan
Copy link
Contributor Author

jjmachan commented Jan 23, 2023

After talking @aarnphm, other than these comments, need to make the following changes

  • run_script should only write to find right before execution to minimize scope for script injection attacks
  • Plan out the interface for conda, docker and other environments
  • Add a caching mechanism to optimize disk usage and env init times. Only create new envs if the dependencies in bentofile.yaml changed Caching will not be implemented for now.

@jjmachan jjmachan force-pushed the feat/env-manager branch 2 times, most recently from 74f56af to c0be259 Compare February 1, 2023 13:13
src/bentoml/_internal/env_manager/envs.py Outdated Show resolved Hide resolved
src/bentoml/_internal/env_manager/manager.py Show resolved Hide resolved
docs/source/guides/envmanager.rst Outdated Show resolved Hide resolved
docs/source/guides/envmanager.rst Outdated Show resolved Hide resolved
docs/source/guides/envmanager.rst Outdated Show resolved Hide resolved
docs/source/guides/envmanager.rst Outdated Show resolved Hide resolved
src/bentoml/_internal/configuration/__init__.py Outdated Show resolved Hide resolved
src/bentoml_cli/env_manager.py Outdated Show resolved Hide resolved
src/bentoml_cli/env_manager.py Outdated Show resolved Hide resolved
src/bentoml_cli/env_manager.py Outdated Show resolved Hide resolved
@jjmachan jjmachan force-pushed the feat/env-manager branch 2 times, most recently from bd9aeb9 to 257467e Compare February 2, 2023 04:56
@aarnphm
Copy link
Contributor

aarnphm commented Feb 7, 2023

Hi @jjmachan, any updates on this PR?

@jjmachan jjmachan marked this pull request as ready for review February 8, 2023 05:36
@jjmachan jjmachan requested a review from a team as a code owner February 8, 2023 05:36
@jjmachan jjmachan requested review from larme and removed request for a team February 8, 2023 05:36
Jithin James added 8 commits February 8, 2023 11:07
Update src/bentoml_cli/env_manager.py

Co-authored-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>

Update docs/source/guides/envmanager.rst

Co-authored-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>

Update docs/source/guides/envmanager.rst

Co-authored-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>

Update docs/source/guides/envmanager.rst

Co-authored-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>

Update docs/source/guides/envmanager.rst

Co-authored-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>

changes
@jjmachan jjmachan enabled auto-merge (squash) February 8, 2023 06:11
@aarnphm aarnphm self-requested a review February 8, 2023 06:34
docs/source/guides/envmanager.rst Show resolved Hide resolved
src/bentoml/_internal/env_manager/__init__.py Show resolved Hide resolved
src/bentoml/_internal/env_manager/envs.py Show resolved Hide resolved
src/bentoml/_internal/env_manager/envs.py Outdated Show resolved Hide resolved
src/bentoml/_internal/env_manager/envs.py Outdated Show resolved Hide resolved
src/bentoml/_internal/env_manager/envs.py Outdated Show resolved Hide resolved
src/bentoml/_internal/env_manager/envs.py Show resolved Hide resolved
src/bentoml/_internal/env_manager/envs.py Outdated Show resolved Hide resolved
src/bentoml_cli/env_manager.py Show resolved Hide resolved
src/bentoml_cli/env_manager.py Outdated Show resolved Hide resolved
aarnphm and others added 5 commits February 8, 2023 00:30
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Co-authored-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Co-authored-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
Signed-off-by: Aaron Pham <29749331+aarnphm@users.noreply.github.com>
@jjmachan jjmachan merged commit ab20ed8 into bentoml:main Feb 8, 2023
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