adding container to update models on a schedule #46
Conversation
…point into model-updater
app/core/app_state.py
Outdated
| @@ -13,7 +13,7 @@ | |||
| from model import Detector | |||
|
|
|||
| from app.core.utils import safe_call_api | |||
|
|
|||
| from .utils import load_edge_config | |||
There was a problem hiding this comment.
Moved this to utils so that the new Dockerfile for model updating won't have to add all sorts of dependencies from edge_inference.py that it does not need, such as the kubernetes client, groundlight, etc.
…oint into model-updater
app/core/utils.py
Outdated
|
|
||
| logger.warning("EDGE_CONFIG environment variable not set. Checking default locations.") | ||
|
|
||
| default_paths = [DEFAULT_EDGE_CONFIG_PATH, "configs/edge-config.yaml"] |
There was a problem hiding this comment.
Do we need configs/edge-config.yaml still?
There was a problem hiding this comment.
For some reason looks like DEFAULT_EDGE_CONFIG_PATH wasn't being recognized. Will look into this closely separately from this PR.
model_updater/update_models.py
Outdated
| if __name__ == "__main__": | ||
| while True: | ||
| update_models() | ||
| time.sleep(3600) |
There was a problem hiding this comment.
We should make this number configurable (via the edge_config file for example) and set the default small, maybe like every 2 minutes (its very cheap to check for a new model).
There was a problem hiding this comment.
(Just repeating my slack comment here)
One question that comes to mind is - do we need a separate dockerfile and image for the model-updater at all?
We already need to download the edge-endpoint image onto the edge device, so if it has the model updating code then we could just run the same image w/ a different entrypoint or command.
This is something we do in zuuul, for example. Same image for janzu and the celery pods, but different commands.
Dockerfile
Outdated
| @@ -55,6 +55,9 @@ RUN mkdir /etc/groundlight/edge-config && \ | |||
| # Copy configs | |||
| COPY configs ${APP_ROOT}/configs | |||
|
|
|||
| # Copy model updating code | |||
| COPY model_updater ${APP_ROOT}/model_updater | |||
There was a problem hiding this comment.
Not sure it makes sense to have model_updater here, and app installed only in the production image, since model_updater depends on app being present. We should probably relocate this below the COPY /app line
…oint into model-updater
model_updater/update_models.py
Outdated
| inference_config = edge_inference_manager.inference_config | ||
|
|
||
| try: | ||
| # All detectors should have the same refresh rate. |
There was a problem hiding this comment.
We should throw an error (or at least log a warning) if they dont, rather than silently ignore all except for the first
Adds a container in the edge deployment that specifically takes care of updating models (every 2 minutes by default).