Refactor the server as a class that returns an object

[matweldon]

Currently, the server is a function that wraps up building a FastAPI instance and serving it using uvicorn.run().

But for operational use, we don't need to serve it, because the user can serve the app (for example in a dockerfile uvicorn cli command with worker settings etc)

The user might also need to add to the FastAPI instance before serving, for example adding HTTP headers, custom logs and authentication middleware.

Proposal

The server module builds the FastAPI object and returns it so that the user can run something like:

from server import ClassifAIServer

server = ClassifAIServer(...
                                      )

app = server.expose_app()
# The app is a FastAPI object
# You can then do uvicorn main:app

# Example, adding HTTP headers
@app.middleware("http")
def add_headers(...):
       pass


# Alternatively to call the python script directly
# Works same as start_api()
server.run()

Maybe it would be possible to subclass FastAPI then add .run() as a method directly. But maybe this is easier

[matweldon]

@frayle-ons @Tom-Owen-ONS @lukeroantreeONS please comment on this issue

[frayle-ons]

I like the separation of ClassifAIServer.__init__ and server.run()

Not sure what the best way to set additional behaviours on the app is, like middleware. Possibly exposing the app object means the user would be able to overwrite or break our precoded rules for connecting the endpoints to the vectorstore method - but it seems like it would give the user a lot of freedom to configure the fastapi how they like. Alternatively, passing a config object that contains middleware to the __init__ would mean we would need to hardcode the logic in the init constructor along with all the other possible config options we might want to include - which could be a lot of options.

Subclassing seems like an interesting idea, wonder if that could solve some of the issues above? Users could specify all the configuration options like middleware first and then pass the vectorstores to .run() which would sync up the endpoints to the vectorstore last and start the server.

[Tom-Owen-ONS]

I think I prefer the idea of a class for this, thought more broadly I agree with the underlying goal that we want to return something composable (e.g. the app object). I think users who choose to do this should probably accept that they run the risk of running conflicting rules/config and we could just document it? Like “If you modify/remove our routes, behaviour is undefined/unsupported.”.

I think one thing I could suggest is to make this class have functionality to expose a router, instead/as well as the app object, so users can just mount the generated endpoints into their existing app in a FastAPI-friendly way. This is how I personally would use this if I was to implement into ScannerAI. I think it would look something like this:

from fastapi import APIRouter, FastAPI

class ClassifAIServer:
    def __init__(self, vector_stores, endpoint_names, prefix: str = ""):
        self._vector_stores = vector_stores
        # ...and the rest

    def create_router(self):
        router = APIRouter(prefix=self._prefix)
        # ... make endpoints and register them like
        @router.post(f"/{endpoint_name}/search", description=f"{endpoint_name} search endpoint")
        async def search_endpoint(...):
            # etc
        return router

    def expose_app(self, app):
        # still have the ability to return the app(that simply uses the router to reduce duplication) for those who want it
        app = app or FastAPI()
        app.include_router(self.create_router())
        return app

    def run(self, host="127.0.0.1", port=8000, log_level="info"):
        uvicorn.run(self.expose_app(), host=host, port=port, log_level=log_level)

Users could then just wire this router into their app to get the endpoints:

from fastapi import FastAPI
from classifai.server import ClassifAIServer

app = FastAPI(...)
server = ClassifAIServer(...
                                      )

router = server.create_router()

# Example, adding HTTP headers
@app.middleware("http")
def add_headers(...):
       pass

app.add_middleware(SomeCustomMiddleware)
app.include_router(router, prefix="/embeddings")

[Tom-Owen-ONS]

At the moment I can't see much in the start_api() function that depends on a tight FastAPI configuration (could very much be wrong here though!), but if we wanted to be explicit and potentially restrict people's options here, we could use a config object of some kind, like a Pydantic model or a dataclass to have a bit of runtime validation in there? Pydantic itself uses a TypedDict for config, but I think we're potentially suggesting something that has a bit more validation built in here, though might be misunderstanding slightly.

[lukeroantreeONS]

@matweldon - with the approach you outlined,

app = server.expose_app()

@app.middleware("http")
def add_headers(...):
       pass

server.run()

,
I think it's creating the app & defining the endpoints, then applying middleware on top - is that the case?

If so, it could introduce a few limitations to how people could configure it - e.g. most standard middleware which involves interacting with request objects directly (e.g. IP checking / authentication / rate-limiting / etc.) need to be applied to a method before the FastAPI decorator converts it to an endpoint.
i.e., for some tasks a user would need (effectively)

app.post('/endpoint_name')
app.middleware(...)
def endpoint_method(...)
    ...

but would be limited to

app.middleware(...)
app.post('/endpoint_name')
def endpoint_method(...)
    ...

(with the app.post() being applied first).

If I've understood things right, I think @Tom-Owen-ONS's suggestion would allow that functionality (although I haven't experimented with APIRouter myself). That said, I'd prefer them to not have to interact with FastAPI directly as in Tom's suggestion unless they choose to.

So my recommendation would be classifai.server offers the following (listed in the order they would be applied):

• .initialise_app() - [required] - creates and returns a FastAPI app instance, and a list of the planned REST API endpoints - without creating them.
• .set_middleware() - [optional] - add any user-specified middleware to the endpoint methods.
• .add_routing() - [required] - link the endpoint methods to the planned REST API endpoints.
• .run() - [optional] - start a uvicorn server exposing the app. Optional, as the user may wish to start it externally with script_name:app using uvicorn / gunicorn / other.

Note: if we go with Tom's suggestion for a dedicated class, the .initialise_app() I suggested could be rolled into the __init__() method, and the others could reference self.app where required. So this flow/pattern can be agnostic of the methods / class decision.

[Tom-Owen-ONS]

Ah yeah @lukeroantreeONS yeah I think that's why still having the run() function/method could be good so that users don't need to interact with FastAPI at all, like you said. I don't have strong opinions on the class vs module function approach to this, but perhaps the router could be made as part of the __init__() method, thereby generating and storing the endpoints then the app object itself is only created/run on a .run() or app exposing method.

In terms of the middleware ordering thing... I'm pretty sure when you add middleware to the app it applies to the app level, rather than individual routes and that the only ordering that matters is the relative ordering of individual middleware rather than whether the routes existed first (in scanner we register logging middleware separate to frontend but it still all ties together neatly).

Do you mean things like endpoint function decorators, like @rate_limit etc? The FastAPI docs mentions doing things like authentication (amongst other things) for larger apps by adding dependencies to the APIRouter object: https://fastapi.tiangolo.com/tutorial/bigger-applications/#dependencies. In this case each endpoint would be wrapped with the dependencies. Would be good to look at things like Depends objects and stuff but I think it's beyond the scope of an initial PR/implementation of this idea.

[Tom-Owen-ONS]

@lukeroantreeONS I just had a thought that I might have misinterpreted re: middleware and you might have been meaning if user continue to use a ClassifAIServer(...).run() or start_api() like function that serves the app (regardless of if using a class method or module function), in which case you wouldn't be able to add middleware after the fact because the process blocks and the server is already running. I guess for users that want that functionality we could pass the FastAPI app object from server.expose_app() as an optional key word argument to the .run() method like this:

server = ClassifAIServer(...)
app = server.expose_app()
# add middleware here
server.run(app=app)