running `uv run pytest` on laptop breaks backend python enviroment in developer mode

aittalam commented 1 month ago

Describe the bug In developer mode, the lumigator folder is mounted on the backend container. The fastapi server automatically reloads whenever any change in the backend folder is detected, so developers can update code in their IDE and see changes reflected immediately on the backend server.

Since the introduction of uv, the backend python environment is stored in /mzai/lumigator/python/mzai/backend/.venv and is generated automatically when the backend is deployed with docker compose via the make local-up command. Things work properly until the environment is updated locally (e.g. on laptop) via e.g. uv sync or uv run pytest: when the command is run, the previous .venv (which points to a python interpreter which does not exist on laptop) is rebuilt and thus updated on the backend too. At that point, fastapi reloads the server but then -probably because it finds a .venv which is now unsuitable for the backend- shuts down the server.

System Information (please complete the following information):

OS: macOS Sonoma
Lumigator version: 543a3a1

To Reproduce

> cd /tmp
> git clone https://github.com/mozilla-ai/lumigator.git
> cd lumigator
> ll lumigator/python/mzai/backend/.venv

ls: lumigator/python/mzai/backend/.venv: No such file or directory

> make local-up
[...]
 => [backend stage-0 5/5] RUN uv sync --frozen
 => => # Using CPython 3.11.10 interpreter at: /usr/local/bin/python
 => => # Creating virtual environment at: .venv
[... everything's up and running]

> test_dataset_upload

Connecting to http://localhost:8000...
{
  "id": "b80c3a52-8e64-4eec-b11e-f858697f061a",
  "filename": "thunderbird_gt_bart.csv",
  "format": "experiment",
  "size": 180528,
  "ground_truth": true,
  "created_at": "2024-10-24T10:32:51"
}
[the API is there, things work properly]

> cd lumigator/python/mzai/backend/
> ls -la
...
drwxr-xr-x   9 mala  wheel     288 24 Oct 11:31 .venv
...

[note that below here I am running tests without specifying the SQLALCHEMY_DATABASE_URL. It does not matter whether tests actually run to completion (one could as well run uv sync here), it's just the uv venv setup that breaks stuff]
> uv run pytest

warning: Ignoring existing virtual environment linked to non-existent Python interpreter: .venv/bin/python3 -> python
Using CPython 3.11.5 interpreter at: /Users/mala/miniconda3/bin/python
Removed virtual environment at: .venv
Creating virtual environment at: .venv
   Built schemas @ file:///private/tmp/lumigator/lumigator/python/mzai/schemas
Installed 103 packages in 282ms

[in the meantime on the backend:]

2024-10-24 11:35:28 WARNING:  WatchFiles detected changes in '.venv/lib/python3.11/site-packages...
[... every single file in .venv]
Reloading...
2024-10-24 11:35:28 INFO:     Shutting down
2024-10-24 11:35:28 INFO:     Waiting for application shutdown.
2024-10-24 11:35:28 INFO:     Application shutdown complete.
2024-10-24 11:35:28 INFO:     Finished server process [32]

[back to my laptop]

> ls -la
...
drwxr-xr-x@  8 mala  wheel     256 24 Oct 11:35 .venv
...
[venv has been updated]

> test_dataset_upload

Connecting to http://localhost:8000...
[hangs because the server is down]

Expected behavior One should be able to run tests without breaking the backend or, more precisely, i.e. without updating the .venv that is used in the backend. This has another advantage, i.e. that by preserving the backend venv in place we wouldn't have to re-download dependencies every time we do make local-up.

This could be attained by running pytest in an isolated environment:

SQLALCHEMY_DATABASE_URL=sqlite:///local.db uv run --isolated pytest

I think updating documentation making the dependency between local and backend environments for developers more explicit and adding the above command for tests would already be a reasonable fix.

agpituk commented 1 month ago

Hello there! I've been looking into possible options here, I want to present a possible solution I created this PR (I'll create a proper PR with all the comments after we decided if we like it)

I've replaced the volume in docker-compose with a sync directive. It's based on the example uv has on their repo

I've applied the following changes:

docker-compose doesn't run in detached mode (it's not compatible with watch mode).
Any change done in the backend folder is automatically synced into the container with the exception on what you add into backend/.venv
Any changes made inside the container are not replicated into the local computer
Any changes made to pyproject.toml will result into the container being rebuilt (in case you add new dependencies)

I've tested the flow described in the issue and the container works as expected. Please have a look into it as it changes your flow (your current terminal won't be detached)

PS: I have found though, that even having the .venv ignored, running the tests syncs some files inside the container (I assume that the test will create some temp files that get deleted, because I wasn't able to find them after running pytest

agpituk commented 2 weeks ago

Hi there @aittalam , do you think we can close this one after https://github.com/mozilla-ai/lumigator/pull/323?

aittalam commented 2 weeks ago

Yes @agpituk , thanks! There is a follow-up to this I have tracked in https://github.com/mozilla-ai/lumigator/issues/375

mozilla-ai / lumigator

running `uv run pytest` on laptop breaks backend python enviroment in developer mode #315