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: bentoml.bentos.build changes the cwd when passing build_ctx #3403

Closed
fwindolf opened this issue Jan 9, 2023 · 4 comments
Closed

bug: bentoml.bentos.build changes the cwd when passing build_ctx #3403

fwindolf opened this issue Jan 9, 2023 · 4 comments
Labels
bug Something isn't working help-wanted An issue currently lacks a contributor

Comments

@fwindolf
Copy link

fwindolf commented Jan 9, 2023

Describe the bug

After running bento.bentoml.build and passing a build_ctx argument the cwd is changed to the build_ctx directory.

I would have expected to return to cwd set before that after finishing the build.

To reproduce

build_ctx = tempfile.TemporaryDirectory(prefix=f"bento_{MODEL_NAME}__")

cwd = os.getcwd()

bento = bentoml.bentos.build(..., build_ctx=build_ctx)

assert cwd == os.getcwd(), "Bentoml changed cwd"

Expected behavior

No response

Environment

bentoml: 1.0.12
python: 3.9.10
platform: ubuntu (WSL2)

@fwindolf fwindolf added the bug Something isn't working label Jan 9, 2023
@aarnphm
Copy link
Contributor

aarnphm commented Jan 9, 2023

I believe this is not a bug, as we are aware of this, from this PR #2737.

@bojiang iirc the reason for changing the cwd to make the imports order from service.py works correctly.

We will have to try importing the service when creating the bento, therefore it is considered as a standalone_load.

Maybe we want to fix this behaviour. Probably want to do something under

def build(

try:
	build_config = ...
finally:
	restore_cwd

But this would be a breaking change?

cc @sauyon for thoughts on this as well.

@fwindolf
Copy link
Author

fwindolf commented Jan 9, 2023

Maybe some more context: I use wandb and build the bento directly after training it. This leads to wandb failing on finish with an error, due to the cwd being set to the build_ctx directory, which is deleted after building.

As a user, this seems odd as there is no indication in build that the cwd is changed.

@sauyon
Copy link
Contributor

sauyon commented Jan 9, 2023

Er, this looks like a bug to me; the point of the standalone_load change was to prevent this behavior.

@sauyon
Copy link
Contributor

sauyon commented Jan 12, 2023

We've discussed this internally and have decided this is indeed a bug that we'd like to fix, but since we now want only one Bento to be loaded per process to work around some limitations of the way we're (ab)using Python to load services, we want to wrap every build in its own process; contribution here is welcome!

There would be two changes and it should be pretty straightforward:

  1. Switch the BentoML CLI (
    def build(build_ctx: str, bentofile: str, version: str) -> None: # type: ignore (not accessed)
    ) from using the bentoml.build API to using the internal Bento.create API.
  2. Switch bentoml.build to use subprocess to call out to the bentoml CLI; see bentoml.serve (
    def serve(
    ) for an example of this.

@aarnphm aarnphm added the help-wanted An issue currently lacks a contributor label Jan 19, 2023
@aarnphm aarnphm closed this as not planned Won't fix, can't repro, duplicate, stale Mar 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help-wanted An issue currently lacks a contributor
Projects
None yet
Development

No branches or pull requests

3 participants