Skip to content
This repository has been archived by the owner on Feb 25, 2024. It is now read-only.

fix: Copy models in bento before build #214

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

michaelromagne
Copy link

Description

bentoctl version 0.4.0 is bugged as raised in two issues recently :

This is due to the context manager not being passed to the operator somehow, so the temporary file system is not found :

@contextmanager
def _prepare_bento_dir(self) -> t.Generator[str, None, None]:
assert self.bento is not None
with fs.open_fs("temp://") as temp_fs, fs.open_fs(self.bento.path) as bento_fs:
fs.mirror.mirror(bento_fs, temp_fs)
models_fs = temp_fs.makedirs("models", recreate=True)
for model_info in self.bento.info.models:
model = get_model(model_info.tag)
model_fs = models_fs.makedirs(model_info.tag.path())
fs.mirror.mirror(model.path, model_fs)
yield temp_fs.getsyspath("/")

Going back to version 0.3.4 does not work as the models are not copied at all in the bento before deployment on EC2 using kubectl.

As a quick fix I propose to replace the context manager by a permanent copy of models in the bento that will be deployed.

Copy link

codecov bot commented Dec 5, 2023

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (3c1e08c) 58.92% compared to head (5acffe0) 58.86%.

Files Patch % Lines
bentoctl/deployment_config.py 16.66% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #214      +/-   ##
==========================================
- Coverage   58.92%   58.86%   -0.07%     
==========================================
  Files          24       24              
  Lines        1266     1264       -2     
==========================================
- Hits          746      744       -2     
  Misses        520      520              
Flag Coverage Δ
unit-tests 58.86% <16.66%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@michaelromagne
Copy link
Author

@aarnphm any chance you could review this one ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant