conda-incubator / conda-store

Data science environments, for collaboration. ✨
https://conda.store
BSD 3-Clause "New" or "Revised" License
144 stars 49 forks source link

[ENH] - Enable hot reloading when developing #901

Open peytondmurray opened 4 days ago

peytondmurray commented 4 days ago

Feature description

Currently, development with docker compose up --build can be slow because making a small change requires the developer to docker compose down, then docker compose up --build, which redownloads the entire environment.

uvicorn has support for hot reloading (although there are caveats), and if we can mount the current filesystem into the container read/write, we should be able to develop and have the server hot reload when changes are made.

Value and/or benefit

Faster development.

Anything else?

No response

peytondmurray commented 4 days ago

CondaStoreServer inherits from traitlets.config.Application. Currently we have


class CondaStoreServer(traitlets.config.Application):
...

    def start(self):
        fastapi_app = self.init_fastapi_app()

        uvicorn.run(
            fastapi_app,
            host=self.address,
            port=self.port,
            workers=1,
            proxy_headers=self.behind_proxy,
            forwarded_allow_ips=("*" if self.behind_proxy else None),
            reload=self.reload,
            reload_dirs=(
                [os.path.dirname(conda_store_server.__file__)]
                if self.reload
                else []
            ),
        )

When hot reloading, uvicorn requires the user to pass a string that returns a FastAPI app i.e.:

    uvicorn.run(
        "conda_store_server._internal.server.app:CondaStoreServer.create_webserver", # <-- this factory command instantiates the object which knows the server address and port
        host=server.address, # <-- There's no way for the new CondaStoreServer instance to pass the address and port here
        port=server.port,
        workers=1,
        proxy_headers=server.behind_proxy,
        forwarded_allow_ips=("*" if server.behind_proxy else None),
        reload=server.reload,
        reload_dirs=(
            [os.path.dirname(conda_store_server.__file__)]
            if server.reload
            else []
        ),
        factory=True
    )

However in our case, the traitlets application contains the server address, port, and other settings. There's no way to pass that to the uvicorn.run call without instantiating the traitlets application twice.


So I tried that: if I break out the running of the webserver into conda_store_server._internal.server.__main__:

import uvicorn
import os
import conda_store_server
from conda_store_server._internal.server.app import CondaStoreServer

main = CondaStoreServer.launch_instance

if __name__ == "__main__":
    server = main()
    uvicorn.run(
        "conda_store_server._internal.server.app:CondaStoreServer.create_webserver",
        host=server.address,
        port=server.port,
        workers=1,
        proxy_headers=server.behind_proxy,
        forwarded_allow_ips=("*" if server.behind_proxy else None),
        reload=server.reload,
        reload_dirs=(
            [os.path.dirname(conda_store_server.__file__)]
            if server.reload
            else []
        ),
        factory=True
    )

and define

class CondaStoreServer(Application):
...
    @staticmethod
    def create_webserver() -> FastAPI:
        server = CondaStoreServer.launch_instance()
        return server.init_fastapi_app()

then from what I can see, running conda_store_server._internal.server.__main__ would

  1. Create an instance of CondaStoreServer, which would read configuration values
  2. Call uvicorn.run, which would instantiate another CondaStoreServer via CondaStoreServer.create_webserver():

    • CondaStoreServer.create_webserver()
    • CondaStoreServer.launch_instance() (which would then call CondaStoreServer.start(), initializing the FastAPI app that would be used by uvicorn.

In any case the webserver never seems to come up. It's hard to say what's going wrong, but is there a more natural way of having a FastAPI app factory that also somehow passes settings to uvicorn?

Image

trallard commented 20 hours ago

I might be wrong, but AFAIK, to enable hot reload, it would be needed to use bind mounts on our docker-compose.yamland perhaps run this with docker compose up (might be ok with the --build flag)

Using bind mounts would basically enable hot-reloading. Right now, we are copying the code and installing it in editable mode at build time (for the Docker container). so basically we'd need to add something like:

volumes:
  - type: bind
    source: .
    target: /opt/conda-store (I think?)

without setting those bind mounts uvicorn wouldn't be aware of any changes and the uvicorn.run(..., reload=True) becomes useless which I believe is what is being described in your findings above

peytondmurray commented 10 hours ago

👆 Bind mounts are enabled (with shorthand syntax):

- ./conda-store-server/:/opt/conda-store-server/:delegated

Fortunately @soapy1 has had some insight into getting this working, although recent changes have a few issues that we're still working out.