Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Route simplication #1059

Closed
dave42w opened this issue Nov 29, 2024 · 0 comments · Fixed by #1060
Closed

Route simplication #1059

dave42w opened this issue Nov 29, 2024 · 0 comments · Fixed by #1060
Labels
enhancement New feature or request

Comments

@dave42w
Copy link
Contributor

dave42w commented Nov 29, 2024

I have been exploring the current chain of activity around setting up routes from from creating the Robyn app instance to starting the server. Details are at the bottom of this issue.

In following this chain I believe the Robyn.add_route method (called from the various decorators eg @app.get("/")) is doing too much and too early.

My simplification is to achieve the following

  1. Zero API changes for developers of Robyn apps
  2. More flexibility for developers of Robyn apps. They should be able to assemble routes, routers, middleware etc in any order before the server is started. Because we try to do all the work with routes from the @get decorator we have made it more complicated to handle the developer adding routes, subrouters, middleware etc in different orders.
  3. Simpler code that is more aligned with SOLID principals

So this is the refactor I'm suggesting
:

  1. Robyn.add_route only calls the master (router created inside the Robyn instance) Router.add_route
  2. Router.add_route gets called by Robyn when adding @get at the app level and directly when adding @get to a subrouter (or if you add routes without using the decorator).
  3. Router.add_route should do the minimum to add the route to its routes: List[Route] (ensuring that everything only known at this point is done).
  4. We move all the processing of routes (OpenApi, Middleware, dealing with nesting, introspecting the handler etc) to app.start() where currently it calls get_routes. To do this we have a single prepare_routes method in the Router to do all the steps currently triggered by add_route and end up with a full list of routes ready to be passed to the spawning of processes. This makes it easier to load OpenAPI only when it is enabled. It also means that we can nest Routers to any level (prepare_routes calls itself for all nested routers (so we don't need the subrouter class - it can be left deprecated and empty for compatibility). The only special thing about the master/top-level router is that it is automatically created inside the app Robyn instance.

First, this is my understanding of how routes work at present. I would appreciate it if this could be checked.

  1. Robyn instance created
from robyn import Robyn

app = Robyn(__file__)

At this point in __init__.py the app instance of Robyn is initialised to contain an instance of Router() (from router.py)

self.router = Router()

We have app.router which contains:

self.routes: List[Route] = []

Also in router.py we have the definition of Route as

class Route(NamedTuple):
    route_type: HttpMethod
    route: str
    function: FunctionInfo
    is_const: bool
  1. Next we add a route to the app.
@app.get("/")
def h(request):
    return "Hello, world"

the get decorator is in Robyn in __init_.py

def get(
        self,
        endpoint: str,
        const: bool = False,
        auth_required: bool = False,
        openapi_name: str = "",
        openapi_tags: List[str] = ["get"],
    ):
        """
        The @app.get decorator to add a route with the GET method

        :param endpoint str: endpoint for the route added
        :param const bool: represents if the handler is a const function or not
        :param auth_required bool: represents if the route needs authentication or not
        :param openapi_name: str -- the name of the endpoint in the openapi spec
        :param openapi_tags: List[str] -- for grouping of endpoints in the openapi spec
        """

        def inner(handler):
            self.openapi.add_openapi_path_obj("get", endpoint, openapi_name, openapi_tags, handler)

            return self.add_route(HttpMethod.GET, endpoint, handler, const, auth_required)

        return inner

BUG: The configuration flag self.config.disable_openapi is not respected by Robyn which always creates an OpenAPI instance
BUG: @get (and all other decorators) calls self.openapi.add_openapi_path_obj again configuration flag self.config.disable_openapi is not respected
BUG the return type is not given

Router.add_route is called. This is a very complex function with very deep nesting. I understand the two wraps for sync and async. I have not yet worked out what the wrapped_handler is doing. It needs refactoring as it is unreadable.

The key focus of these methods is a side effect by which the route added to the Robyn.router.routes list

  1. The app is started app.start(port=8080, host="0.0.0.0")

This calls Robyn.router.get_routes to pass the List[Route] to run_processes (processpool.py), the List[Route] gets passed to init_processpool which passes List[Route] to spawn_process

Spawn_process makes a copy of all the routes by adding them individually to server =Server() which is defined in server.rs

It seems that within server.rs the List[Route] is cloned multiple times.

Still to do.

  • Add what happens for views and subrouters
  • add middleware and websockets
@dave42w dave42w added the enhancement New feature or request label Nov 29, 2024
@dave42w dave42w mentioned this issue Nov 29, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant