Codecov Report

Merging#112 intomaster willdecrease coverage by0.03%.
The diff coverage is97.05%.

@@            Coverage Diff             @@##           master     #112      +/-   ##==========================================- Coverage     100%   99.96%   -0.04%==========================================  Files         124      125       +1       Lines        2987     3019      +32     ==========================================+ Hits         2987     3018      +31- Misses          0        1       +1

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last update2ddb804...2996a5a. Read thecomment docs.

Codecov Report

Merging#112 intomaster willnot change coverage.
The diff coverage is100%.

@@          Coverage Diff           @@##           master   #112    +/-   ##======================================  Coverage     100%   100%            ======================================  Files         170    175     +5       Lines        4122   4261   +139     ======================================+ Hits         4122   4261   +139

Continue to review full report at Codecov.

Legend -Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing data
Powered byCodecov. Last updated03678d...01248bd. Read thecomment docs.

First, thanks for your work@euri10 !

I have a couple of concerns:

The compiled JS code is almost 1 MB, and at each new Swagger UI version, we would need to update it. And the changes would be huge, as it is not the source code but the "compiled" (transpiled/minified) version.

I think one way to solve it could be to add a script for building the package, that pulls the lastest JS version from NPM (or any CDN) and adds it to the dist files. But I'm still not completely sure how to make Flit "bundle" it, and then how to add it/serve it in FastAPI, being a file included as part of thefastapi package itself.

Another option would be to create an additional optional package, that includes those static JS files, and can be installed and used with FastAPI.

Let's keep this on hold, I'll investigate what options do we have, and what's the best/most clean way to solve this use case.

I'm not sure I understand your concern, I surely miss something :)

It's just an option users will have:

• either passing nothing akaapp=FastAPI() which defaults to the cdn version of the files.
• either pass astatic_directory kwarg and a dict with the 3 relevant files akaapp = FastAPI(static_directory=static_directory, swagger_static={ "js": swagger_static_js, "css": swagger_static_css, "favicon": swagger_static_icon,},)

If the css / js changes today, you'll update the 2 links in the docs.py file:

https://github.com/tiangolo/fastapi/blob/08d849d5c588f9cf14a38ede9e9d4d0b29a6d9c2/fastapi/openapi/docs.py#L10
https://github.com/tiangolo/fastapi/blob/08d849d5c588f9cf14a38ede9e9d4d0b29a6d9c2/fastapi/openapi/docs.py#L21

Should that option be implemented you'll change them in the spot they're defined by default, I don't see why you'd need to ship them with the library

Ah! Sorry, I was misunderstanding something.

There's an actual copy of the Swagger UI JS bundle in the tests. I was thinking that it was meant to be included as the default static version.

As this approach would still require users (developers) to download and configure the static files by hand and then pass additional parameters to the app object, it would be quite some effort to set everything up. So, most of the effort required in the original issue#110 would still be needed.

I think I would prefer to:

• Document how to serve static.
• Document how to send HTML responses.
• Document how to combine the two to override the docs.

Then, in the case of#110, to reduce as much as possible the steps to get static local docs, I think it could be done with an external installable Python package that includes those static files and can be connected somehow to the app. But I'm not sure yet about all the details.

There is already a python package for swagger ui:https://pypi.org/project/swagger-ui-py

Thanks@wshayes ! Very interesting.

I see it has official support for several specific frameworks. Although it seems maybe some of its utils could be re-used.

It seems to be very close to what I think we could use, the drawback I see is that it's several versions behind. But still, it seems promising.

Good job@euri10 , now it's just removing the extra files intests/static and simplifying the tests.

There's no need to have permutations. Just test that the function without parameters returns the fixed URLs, and that adding parameters, they are included in the response.

Then we're good to go with this. The next step will be documenting how to use it, but that will be later.

Good job@euri10 , now it's just removing the extra files intests/static and simplifying the tests.

There's no need to have permutations. Just test that the function without parameters returns the fixed URLs, and that adding parameters, they are included in the response.

Then we're good to go with this. The next step will be documenting how to use it, but that will be later.

done
I used the function signature to extract defaults so that tests will survive a modification

Thanks@euri10 ! 🎉 🚀

BTW, nice job refactoring with f-strings 😉 🍰

It seems like whileswagger_js_url,swagger_css_url, andswagger_favicon_url are parameterized inget_swagger_ui_html, there isn't a way to influence these values from creating aFastAPI instance. Should we perhaps passself.extra into the inner function athttps://github.com/tiangolo/fastapi/blob/master/fastapi/applications.py#L103 ?

(and similarly for redoc)