Allow empty routed path (issue #414)

[vitalik]

With this patch it should be possible now to include empty route path
it also checks if both path and prefix are not empty
see #414 for more details

[vitalik]

hi

do you think this will land to release ? (we are just trying to understand our project strategy... )

[pablogamboa]

This feature also interests me quite a lot! Perhaps not this exact implementation, but I think it would be very beneficial to have the flexibility of having routes without a trailing slash 👍

[euri10]

I don't like the idea of having FastAPI behaving differently than Starlette, if this should land it should be upstream imho, or not at all. Le jeu. 8 août 2019 à 8:03 AM, Pablo Marti <notifications@github.com> a écrit :

…

This feature also interests me quite a lot! Perhaps not this exact implementation, but I think it would be very beneficial to have the flexibility of having routes without a trailing slash 👍 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#415?email_source=notifications&email_token=AAINSPXFXBGL75EHAPQALY3QDPAKLA5CNFSM4IIOOMH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD32VLUI#issuecomment-519394769>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAINSPXXUIKMNIV5JMB66T3QDPAKLANCNFSM4IIOOMHQ> .

[pablogamboa]

yeah you're right @euri10 the discussion should be moved there

[vitalik]

I don't like the idea of having FastAPI behaving differently than Starlette, if this should land it should be upstream imho, or not at all. Le jeu. 8 août 2019 à 8:03 AM, Pablo Marti
Hi @euri10 , @pablogamboa

Well you forgetting that Starlette's endpoints can handle multiple http methods (GET, POST, HEAD, PUT) with one function, while what nice about fastapi - each method handled in separate function...

So I would not inherit ALOT Starlette's' approach (well I actually question them for not allowing ending slashes as well)

Just take a look at this example API structure:

GET  /cats
POST /cats
GET  /cats/{id}/children
POST /cats/{id}/children
GET  /cats/{id}/tasks
POST /cats/{id}/tasks
GET  /cats/{id}/activities
POST /cats/{id}/activities
GET  /cats/{id}/food
POST /cats/{id}/food
GET  /dogs
POST /dogs
GET  /dogs/{id}/children
POST /dogs/{id}/children
GET  /dogs/{id}/tasks
POST /dogs/{id}/tasks
GET  /dogs/{id}/activities
POST /dogs/{id}/activities
GET  /dogs/{id}/food
POST /dogs/{id}/food
GET  /birds
POST /birds
GET  /birds/{id}/children
POST /birds/{id}/children
GET  /birds/{id}/tasks
POST /birds/{id}/tasks
GET  /birds/{id}/activities
POST /birds/{id}/activities
GET  /birds/{id}/food
POST /birds/{id}/food

if fastapi allowed me to to use empty route - it would lead just 3 lines of code:

app.include_route(..., prefix='/cats')
app.include_route(..., prefix='/dogs')
app.include_route(..., prefix='/birds')

without empty path in route I will have to do the following:

app.get('/cats', cats_get)
app.post('/cats', cats_post)
app.include_route(..., prefix='/cats')

app.get('/dogs', dogs_get)
app.post('/dogs', dogs_post)
app.include_route(..., prefix='/dogs')

app.get('/birds', birds_get)
app.post('/birds', birds_post)
app.include_route(..., prefix='/birds')

not only it produces 3 times more code, but also my views will have weird functions cats_get, cats_post without decorators

how else I would make it nicer ?

[euri10]

You could use a router with prefix cats, one with prefix dogs, etc, Sorry for the formatting I'm on mobile, but in essence with the structure you mentioned you could do what's described here https://fastapi.tiangolo.com/tutorial/bigger-applications/ dogsrouter = APIRouter() catsrouter = APIRouter() app.include_router(dogsrouter) app.include_router( catsrouter, prefix="/cats", ) @catsrouter.get('/') @catsrouter.post('/') @catsrouter.get('/{id}/children') Le jeu. 8 août 2019 à 12:41 PM, Vitaliy Kucheryaviy < notifications@github.com> a écrit :

…

I don't like the idea of having FastAPI behaving differently than Starlette, if this should land it should be upstream imho, or not at all. Le jeu. 8 août 2019 à 8:03 AM, Pablo Marti Hi @euri10 <https://github.com/euri10> , @pablogamboa <https://github.com/pablogamboa> Well you forgetting that Starlette's endpoints can handle multiple http methods (GET, POST, HEAD, PUT) with one function, while what nice about fastapi - each method handled in separate function... So I would not inherit ALOT Starlette's' approach (well I actually question them for not allowing ending slashes as well) Just take a look at this example API structure: GET /cats POST /cats GET /cats/{id}/children POST /cats/{id}/children GET /cats/{id}/tasks POST /cats/{id}/tasks GET /cats/{id}/activities POST /cats/{id}/activities GET /cats/{id}/food POST /cats/{id}/food GET /dogs POST /dogs GET /dogs/{id}/children POST /dogs/{id}/children GET /dogs/{id}/tasks POST /dogs/{id}/tasks GET /dogs/{id}/activities POST /dogs/{id}/activities GET /dogs/{id}/food POST /dogs/{id}/food GET /birds POST /birds GET /birds/{id}/children POST /birds/{id}/children GET /birds/{id}/tasks POST /birds/{id}/tasks GET /birds/{id}/activities POST /birds/{id}/activities GET /birds/{id}/food POST /birds/{id}/food if fastapi allowed me to to use empty route - it would lead just 3 lines of code: app.include_route(..., prefix='/cats') app.include_route(..., prefix='/dogs') app.include_route(..., prefix='/birds') without empty path in route I will have to do the following: app.get('/cats', cats_get) app.post('/cats', cats_post) app.include_route(..., prefix='/cats') app.get('/dogs', dogs_get) app.post('/dogs', dogs_post) app.include_route(..., prefix='/dogs') app.get('/birds', birds_get) app.post('/birds', birds_post) app.include_route(..., prefix='/birds') not only it produces 3 times more code, but also my views will have weird functions cats_get, cats_post without decorators how else I would make it nicer ? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#415?email_source=notifications&email_token=AAINSPQ7ADZXL6CD54A4OQDQDQA4ZA5CNFSM4IIOOMH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD33K5ZA#issuecomment-519483108>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAINSPWBQGOGYE4NKOVEGXDQDQA4ZANCNFSM4IIOOMHQ> .

[vitalik]

@euri10 well - it does not work - try it...

you cannot include these endpoints without ending slash

GET  /cats
POST /cats

but making it outside routed views makes code less modular/incapsulated

[euri10]

unless I'm mistaken Starlette redirects you with a 308 if you don't have the trailing slash well I may be missing something obvious you want, sorry if I dont get it :)

…

On Thu, Aug 8, 2019 at 4:19 PM Vitaliy Kucheryaviy ***@***.***> wrote: @euri10 <https://github.com/euri10> well - it does not work - try it... you cannot include these endpoints without ending slash GET /cats POST /cats but making it outside routed views makes code less modular/incapsulated — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#415?email_source=notifications&email_token=AAINSPXMPAGRNE6WTKIDFHTQDQTQDA5CNFSM4IIOOMH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD33YP6Q#issuecomment-519538682>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAINSPXTGG3J23GCWNVHSCTQDQTQDANCNFSM4IIOOMHQ> .

-- benoit barthelet http://pgp.mit.edu/pks/lookup?op=get&search=0xF150E01A72F6D2EE

[steinitzu]

Redirects don't solve this for me. I want my route to be documented as GET /cats not GET /cats/ as it's consistent with my other endpoints that don't have a trailing slash.

What's the reason for not allowing empty paths?

[dmontagu]

@steinitzu I'm assuming it's just a way to ensure you don't accidentally end up with paths that aren't separated by slashes. Either way, to @euri10's point, I think that the right place to ask this question would probably be on starlette because of this: https://github.com/encode/starlette/blob/master/starlette/routing.py#L147

[vitalik]

@dmontagu well starlette routing have nothing to do with fast-API app.get/post/patch.. these a re different code basses

I'm assuming it's just a way to ensure you don't accidentally end up with paths that aren't separated by slashes

my pull request is actually makes this check - if you use empty routed path and include - it will throw an error

[dmontagu]

@vitalik

well starlette routing have nothing to do with fast-API app.get/post/patch

I think that's a little extreme; clearly they are related -- APIRoute is a subclass of starlette.routing.Route after all.

That said, it is a valid point that include_router doesn't exist in starlette, so this concern wouldn't apply. Given APIRoute is typically added to an app via include_router I'm convinced this is a reasonable change. I personally would be fine with merging it.

Either way, it would certainly be less controversial if there was a way to implement this that didn't remove validation that would be typically be performed in the superclass.

[vitalik]

I'm sorry - maybe I am extreme , sorry about that - but this patch is only removing one line with assert inside fastapi code (have nothing to do with starlette) that's it (plus extra check that both include an route are not empty)

if you can give me any other option how to make my code more modular/isolated withot this assert - I would be happy to use it

[vitalik]

Hi @tiangolo
Can you share your concerns or plans about this PR ?

Thank you

[tiangolo]

Thank you @vitalik ! Merged! 🎉 🚀

---

I think I can agree with you all 😄

I agree I don't want to diverge from Starlette, but it's true that the router functionality and the "path operations" are already handled differently.

I think for a new developer to find a path operation with an empty route in an existing codebase could be confusing, and that in some cases it could be more "intuitive" to have paths that are a prefix of other paths always have a trailing slash.

But that's a bit subjective, depending on the point of view, in fact, GitHub itself has by default the contrary behavior, the one that would be supported by this feature (it's https://github.com/tiangolo/fastapi , not https://github.com/tiangolo/fastapi/).

I also see that this is a feature that seems to help several use cases (at least 3, just from this thread), and I think it might help especially with some migrations, to keep paths consistent with previous implementations. Also, the flexibility it adds doesn't break current behavior, so I think it's fine.

Thanks for your contribution @vitalik ! And thanks for the interest and discussion here everyone!

---

Now, although I ran the tests locally first (and they passed), as Travis seems to have not triggered, I see now there's a mypy error after merging 😞. So I'll fix it before a release.