-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
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
Starlette 0.13 declarative support? #687
Comments
FWIW I don’t think FastAPI necessarily needs to consider any changes here - it’s fairly reasonable for higher level frameworks to adopt a different approach, while Starlette itself sticks to a as-plain-as-possible style. (Also I don’t think there’s any changes needed to deal with the 0.12-0.13 update, tho I’m keen to double check that all before I hit go on it) I am interested in folks thoughts about the style change, tho. It’s an odd trade off, because I’m not sure it necessarily looks nicer, but I think it’s a good thing for the project. From my POV it’s a good thing for Starlette since it more simply expresses the mechanics, and is easier to override the behaviour (eg subclassing Route makes sense, whereas that’s more awkward to do if there’s a .route decorator on the app instance). Those are particularly valuable qualities for Starlette since it makes it easier to build different high level styles on top of it. |
@tomchristie Thank you for your insights! I was just opening the debate here so that FastAPI maintainers can start to think about it :) Regarding the style change of Starlette, from my limited API developer point-of-view, I have to say that I tend to prefer the decorator style. I find very handy to have the path, the parameters, the HTTP method and the logic behind at the same place in the code. If I may make the parallel with Django, I'm often frustrated to go back and forth between But I understand your wish to limit the hidden magic behind the framework and to make it easier to extend. |
@tomchristie: I wasn't sure initially because I've already gotten used to the Flask-like decorator magic, but the more I look at it, the more I believe it's a good thing, especially if the intention is to make Starlette an ASGI library rather than a full-on framework. The problem with decorators is that they require the app or router to always be in-scope, which leads to odd patterns in Flask-land to avoid depencency loops. Making routes declarative at initialization time provides both a nice tree-like structure of the routes in the code itself, with very little magic to hide sub-application mount points, route subclasses, the evaluation order for each route, etc. Given those have some pretty important repercussions on how the resulting application will behave, I can definitely appreciate making this a bit more explicit. The main problem I can see if FastAPI were to switch to this new model is that decorators clearly show which functions are intended to be routes and which aren't, which becomes important when you consider how dependency injection and parameters work, since those don't work with "normal" functions, only routes and the dependencies those route call (which are admittedly unmarked as well). Having the route declaration in a different file could be frustrating in that regard, though it might not be a problem in practice. EDIT: What I mean is like how, in the example below, the route ( @app.get("/users/{user_id}", response_model=schemas.User)
def read_user(user_id: int, db: Session = Depends(get_db)):
db_user = crud.get_user(db, user_id=user_id)
if db_user is None:
raise HTTPException(status_code=404, detail="User not found")
return db_user |
Indeed yeah. You could have the |
@tiangolo: Regarding what you said in the starlette issue, FastAPI would also run into a problem with Response models, which are specified in the route decorator to let functions return any object that can be serialized into this type, regardless of the actual type. Assuming that can't be integrated into the route function's signature in some way (maybe using metaclasses?) or moved to a new decorator, switching to a declarative model like this would also separate the type of the function's parameters from the type of its "return value". |
This is a good point I hadn't considered. I too thought that adding another decorator for specifying a subset of the endpoint kwargs might make it more natural to follow the new starlette routing convention. Something like the following: @endpoint(response_model=User, response_class=UJsonResponse)
def get_user(user_id: str):
... The It would have the benefit of signaling that your goal is to use the function as an endpoint call, clearly indicating that FastAPI's dependency injection can be expected to be used. Also, it would simplify the implementation of this CBV pattern for reusing the same dependencies across many routes (admittedly I seem to be the only person who cares about this 😅). It would also make it more natural to implement |
I would love to see class-based views for easier re-use. :) |
My previous work project was a Flask app, and my current work project is a Django app. I really disliked the
The one thing I miss about Flask's decorators is that it can be difficult to tell which methods are supported by a particular route (whereas once you find the decorator, it's right there). |
@onecrayon What do you think about my proposal above for a decorator called The obvious counter argument would be that being opinionated makes it so conventions are consistent across examples. But given the points made above, I think there are some route decorator arguments that should live near the endpoint function definition (e.g., anything related to the response model type), and some that could/should be moved to a routing table (e.g., path). |
@dmontagu Your proposal makes a lot of sense to me. The main benefit I see for the current decorator style is that it tightly couples the endpoint with things like its response model. The reason I like centralized router definitions is because it gives you a single point of entry to navigate the code (super helpful for maintenance). However, I don't like having too much configuration there, because that splits the codebase in non-useful ways (the only data I really want to know at the Router level is "what callable is responsible for handling this route?"). Particularly with class-based views where you might have different strategies for different HTTP methods, being able to define things specific to a given endpoint/method would be preferable. |
any news about update to starlette 0.13? |
As far as I know this should just work with 0.13, as they keep those decorators around? A problem I currently see is that they purge all the decorator stuff from the Starlette documentation, making it impossible to just write a normal app with the decorator support. |
i want to use new route sintax from starlette 0.13 in fastapi |
Either way, it needs documentation how to use both of them. |
Are there references in the current FastAPI documentation to things that no longer exist in the Starlette 0.13 documentation? I don't recall frequently referencing the starlette docs when I was learning FastAPI, so I'm not sure where something should be changed. At some point soon I'll try to throw together a draft PR along the lines I've described above, which would make it easier/possible to use the routing table approach, but I think there is still some question of what is the right thing to even do here is, and I think there is a non-zero chance @tiangolo will disagree with the approach I take. So it might be a little while before this is properly supported in FastAPI. |
Basically how you would do the whole normal Web app with @decorators. In the current state I feel like fast api would be the much better flask alternative, especially after flask's style is praised so much in the introduction. For the long term, I feel like it wouldn't be that bad to keep the decorators around in the FastApi project even if they vanish from Starlette. In the end I'd love to hear @tiangolo's opinion how he'll like to proceed in the future, please. I have to choose my framework for the next project. |
|
even with fastapi/fastapi#687 maybe becoming a problem in the future. Let's all hope I don't regret that decision later.
Thanks for all the discussion here everyone! TL;DR: FastAPI will keep the decorator style. There's a lot of information shared between the decorator and the actual function, for example But nothing prevents you from also using Starlette's new preferred style with FastAPI. ✨ You can use Starlette's style, it also works ✨ (and actually, it has always worked) as FastAPI is more or less just "Starlette on steroids". 🤷♂️ 😅 You just have to use from fastapi import FastAPI
from fastapi.routing import APIRoute
from pydantic import BaseModel
from starlette.responses import JSONResponse
class Item(BaseModel):
name: str
price: float
is_offer: bool = None
def read_root():
return {"Hello": "World"}
def read_item(item_id: int, q: str = None):
return {"item_id": item_id, "q": q}
def update_item(item_id: int, item: Item):
return {"item_name": item.name, "item_id": item_id}
app = FastAPI(
routes=[
APIRoute("/", read_root, methods=["GET"], response_class=JSONResponse),
APIRoute(
"/items{item_id}", read_item, methods=["GET"], response_class=JSONResponse
),
APIRoute(
"/items{item_id}", update_item, methods=["PUT"], response_class=JSONResponse
),
]
) ...feel free to try it out at home ☣️ 😉 I'll also copy here my comment from the thread in Starlette, just for completeness: About the I see how this helps to structure parts by their type, all routing is done in a single place, in a list, and the functions that handle each route are separated in another place. On the other side, my argument in pro of decorator style is that it ties things together by the feature or subject they are related to. So, the code that handles a username is near the declaration of the route I find that especially useful with routes with path parameters like: @app.route('/user/{username}')
def user(request):
username = request.path_params['username']
return PlainTextResponse('Hello, %s!' % username) In this case, the If someone was refactoring that to make it Having those two related texts naming something in different files could, in some cases, make that refactor a bit more difficult. This is similar to the philosophy in React, of having code of different types (HTML-ish JSX and JavaScript) close together, in the same file/component, but related in their subject, not the type of code. And React-router works in a similar way. I actually didn't like React until recently, that I started using it and it made sense to me (I used only Angular and Vue before that). But it also seems that that idea (React itself) ended up being quite popular. The preference between these two can quickly become quite subjective and dependent on personal/team taste, so I understand if this is the preferred way for Starlette. I still wanted to share that additional aspect of it, that might be important. For FastAPI it's quite more relevant, as there's even more "shared information" between the part that declares a route path (the decorator) and the code in the function that handles it, so I'll keep that as the default there. As long as I can keep using decorators in FastAPI, I'm OK with the decision. It currently uses But even if not, if there's a way to update the routes incrementally (I guess some |
Glad we have a clear discussion now. However there is now the problem that Starlette has purged all decorator style from their documentation, and unlike the mature frameworks they don't serve the previous versions of it. Still it often makes sense to be able to have a normal html page, and if only to say "oh hey you hit the api server", therefore that should be documented. So I guess that'd need to be extracted to be part of FastAPI's documentation, to be able to take advantage of Starlette as well. |
Yes, HTML responses should and will be documented in FastAPI, but that's a different subject/issue. |
Assuming the original issue was solved, it will be automatically closed now. But feel free to add more comments or create new issues. |
Are there any plans to upgrade the starlette version in the near future? Starlette is currently locked at |
This issue was moved to a discussion.
You can continue the conversation there. Go to discussion →
Description
In the upcoming release 0.13 of Starlette, a new declarative way for routing is introduced. At first, decorator style will still be supported but @tomchristie will somewhat deprecate it and remove it at some point in the future:
At the light of this, it seems that FastAPI needs to take a decision about this: either follow Starlette new convention or stick with the decorator style.
Personally, I tend to prefer the decorator style, which is similar to Flask or even Node's Express. However, as Starlette is one of the core component of FastAPI, I think it's better if it keeps up with its evolutions, for several reasons:
That's it! Just some quick thoughts in order to keep this subject in mind 🙂
The text was updated successfully, but these errors were encountered: