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.

Sign up for GitHub

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

Jump to bottom

Bug: New create_static_files_router automatically adds routes to schema docs #3374

Closed
3 of 4 tasks
JacobCoffee opened this issue Apr 11, 2024 · 10 comments · Fixed by #3509
Closed
3 of 4 tasks

Bug: New create_static_files_router automatically adds routes to schema docs #3374

JacobCoffee opened this issue Apr 11, 2024 · 10 comments · Fixed by #3509
Labels
area/openapi This PR involves changes to the OpenAPI schema area/static-files Bug 🐛 This is something that is not working as expected Good First Issue This is good for newcomers to take on Help Wanted 🆘 This is good for people to work on

Comments

@JacobCoffee
Copy link
Member

JacobCoffee commented Apr 11, 2024 •
edited

Description

I found this suprising when using the new static files feature that
1: It auto adds these to the schema
2: It adds them under a sort've "ugly" router header default

We should fix one or both of those

URL to code causing the issue

No response

MCVE

from litestar import Litestar
from litestar.static_files import create_static_files_router

app = Litestar(
    route_handlers=[
        create_static_files_router(directories=["assets"], path="/static"),
    ],
)

Steps to reproduce

1. run mcve
2. visit /schema
3. See schema for default

Screenshots

image

Logs

No response

Litestar Version

2.7/2.8

Platform

  • Linux
  • Mac
  • Windows
  • Other (Please specify in the description above)
@JacobCoffee JacobCoffee added the Bug 🐛 This is something that is not working as expected label Apr 11, 2024
@provinzkraut
Copy link
Member

Why wouldn't it add them to the schema? It's just regular route handlers, which, by default, are added to the schema. You can use the regular include_in_schema=False if you don't want them included.

Not sure about the name; We do include a name param which defaults to static. No idea where the default comes from, that might be a bug.

@JacobCoffee
Copy link
Member Author

We didn't include them before, but I feel like - by default - they should be disabled.
The default is a tagless section iirc.

I'm fine with whatever here, but at the very least I hope we would tag this as something more not-default-ey

@peterschutt
Copy link
Contributor

The default is a tagless section iirc.

If you pass tags=["static"] (or whatever you want to call it) to the create_static_files_router() function, does that replace the "default" section? Maybe we should have a default that is not None for that?

@Nozavi
Copy link

Nozavi commented Apr 24, 2024

I, for one, chose to stick with StaticFilesConfig as much as possible just because it does NOT add the routes to the schema docs. Since I'll never use them, I'd rather not see/have them there. So, for what it's worth, I'd also classify this as a bug (especially if you take into consideration that create_static_files_router is presented as the replacement for StaticFilesConfig, but it does not replace/mimic its behavior exactly).

@peterschutt peterschutt added Help Wanted 🆘 This is good for people to work on Good First Issue This is good for newcomers to take on area/openapi This PR involves changes to the OpenAPI schema area/static-files labels Apr 26, 2024
@tibor-reiss tibor-reiss mentioned this issue May 17, 2024
Merged
@peterschutt
Copy link
Contributor

FWIW, I think include_in_schema=False is a sane default for the static files router.

@Nozavi
Copy link

Nozavi commented May 22, 2024 •
edited

I checked again just now and realized there was one additional reason/inconsistency: after switching to create_static_files_router (using the exact same settings I had for StaticFilesConfig: directories=["static"], path="/static") I immediately noticed my favicon was 404ing all of a sudden because its URL was now /schema/static/favicon.ico instead of StaticFilesConfig's /favicon.ico . The new URL didn't make sense to me, so as I previously mentioned, I switched back to StaticFilesConfig...

@provinzkraut
Copy link
Member

I checked again just now and realized there was one additional reason/inconsistency: after switching to create_static_files_router (using the exact same settings I had for StaticFilesConfig: directories=["static"], path="/static") I immediately noticed my favicon was 404ing all of a sudden because its URL was now /schema/static/favicon.ico instead of StaticFilesConfig's /favicon.ico . The new URL didn't make sense to me, so as I previously mentioned, I switched back to StaticFilesConfig...

That would be a bug in StaticFilesConfig though. If you set directories=["static"], path="/static", all files from the static directory should be served under the /static path.

@Nozavi
Copy link

Nozavi commented May 23, 2024

True, but I don't understand/want that "/schema" part/folder from create_static_files_router's URL

@provinzkraut
Copy link
Member

Can you provide an MCVE for this?

@github-actions GitHub Actions
Copy link

github-actions bot commented Jun 2, 2024

A fix for this issue has been released in v2.9.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area/openapi This PR involves changes to the OpenAPI schema area/static-files Bug 🐛 This is something that is not working as expected Good First Issue This is good for newcomers to take on Help Wanted 🆘 This is good for people to work on
Projects
Overview
Status: Closed
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: exclude static file from schema tibor-reiss/litestar
4 participants
@Nozavi @peterschutt @provinzkraut @JacobCoffee