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

Routed paths must always start with / #414

Closed
vitalik opened this issue Jul 31, 2019 · 6 comments
Closed

Routed paths must always start with / #414

vitalik opened this issue Jul 31, 2019 · 6 comments
Labels
answered feature New feature or request reviewed

Comments

@vitalik
Copy link
Contributor

vitalik commented Jul 31, 2019

Hi

Let's say I have the following API:

GET /accounts  - list of accounts
POST /accounts  - create account
GET /accounts/{id} - get account by id
DELETE  /accounts/{id} - delete account

then I would make a module accounts with APIRouter

in main.py

app.include_router(accounts.router, prefix="/accounts")

and in accounts.py

router = APIRouter()

@router.get("")
def list_accounts():
    ...

@router.post("")
def list_accounts():
    ...

@router.get("/{id}")
def get_account(id:str):
    ...

but I'm getting an error "Routed paths must always start with /"

if I add to router "/" - then my resources will look like GET /accounts/ , POST /accounts/ (but I do not want slash at the end...)

Basically the idea is to put 5-10 methods inside module and all should start with /accounts

how should i solve this ?

@vitalik vitalik added the question Question or problem label Jul 31, 2019
@vitalik vitalik changed the title [QUESTION] Routed paths must always start with / Routed paths must always start with / Jul 31, 2019
@euri10
Copy link
Contributor

euri10 commented Jul 31, 2019 via email

@vitalik
Copy link
Contributor Author

vitalik commented Jul 31, 2019

@euri10 yeah, I did - my prefix="/accounts" - but i cannot make api resource /accounts (without ending slash) now with routers

@dmontagu
Copy link
Collaborator

dmontagu commented Jul 31, 2019

I think you'd have to do:

@router.get("/accounts")
def list_accounts():
    ...

@router.post("/accounts")
def list_accounts():
    ...

@router.get("/accounts/{id}")
def get_account(id:str):
    ...

# ######

app.include_router(accounts.router, prefix="")  # can drop the prefix kwarg

You can mount multiple routers with the same prefix, so this won't cause a problem if you want to use a different router for each resource.

You can make it a little more DRY-friendly via:

prefix = "/accounts"

@router.get(prefix)
def list_accounts():
    ...

@router.post(prefix)
def list_accounts():
    ...

@router.get(prefix + "/{id}")
def get_account(id:str):
    ...

# ######

app.include_router(accounts.router, prefix="")  # can drop the prefix kwarg

You'd have to make sure you didn't create conflicts by naming routers the same thing, but that's always a risk anyway.

(I'm not sure whether there is a really good reason for disallowing "" as the routing path, or if that is more of a leaky implementation detail.)

@vitalik
Copy link
Contributor Author

vitalik commented Aug 1, 2019

but what's the technical reason not to allow empty router if parent-include have prefix ?

@jaddison
Copy link
Contributor

It does seem sort of awkward that using prefix prevents 'trailing slash' naked endpoints, as OP mentioned.

Good: /cats
Bad: /cats/

(subjectively speaking)

@tiangolo
Copy link
Member

Thank you, everyone, for the discussion here!

Thanks @vitalik for the PR! More comments on the PR itself: #415 (comment)

You can now use this in FastAPI 0.36.0 with an empty path (""). 🎉 🚀


I'll close this now, but feel free to add more comments or create new issues.

@tiangolo tiangolo added feature New feature or request answered reviewed and removed question Question or problem labels Feb 22, 2023
lmignon added a commit to acsone/fastapi that referenced this issue Sep 19, 2024
From odoo/odoo@cb1d057, the orignal werkzeug request is wrapped in a dedicated class to keep under control the attributes developers use. This change add code to make the fastapi addon working with version including this last change and prior version

refs fastapi#414
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
answered feature New feature or request reviewed
Projects
None yet
Development

No branches or pull requests

5 participants