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

Add Open API prefix route - correct docs behind reverse proxy #26

Merged
merged 11 commits into from Feb 14, 2019

Conversation

@kabirkhan
Copy link
Contributor

@kabirkhan kabirkhan commented Feb 10, 2019

I run a lot of services in Kubernetes using Ambassador as my API Gateway. Ambassador allows you to configure a route prefix for each one of your services and then it handles ingress and routing to the initial service. This PR enables me to configure the same prefix I use with Ambassador in the Swagger UI API docs.

Example:

prefix='/service1'

app = FastAPI(
    openapi_prefix=prefix
)
@app.get('/something') => SwaggerRoute (GET - /service1/something)

NOTE: This is not the same as using the include_router method and adding a prefix.
This is meant only to rewrite the route paths in the Swagger UI.

BTW: Awesome Project! Thanks so much for your work on this, I'm using fastapi a ton these days.

@codecov
Copy link

@codecov codecov bot commented Feb 10, 2019

Codecov Report

Merging #26 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #26   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          95     99    +4     
  Lines        2484   2582   +98     
=====================================
+ Hits         2484   2582   +98
Impacted Files Coverage Δ
fastapi/openapi/utils.py 100% <100%> (ø) ⬆️
...tutorial/test_sub_applications/test_tutorial001.py 100% <100%> (ø)
fastapi/applications.py 100% <100%> (ø) ⬆️
docs/src/sub_applications/tutorial001.py 100% <100%> (ø)
tests/test_jsonable_encoder.py 100% <0%> (ø) ⬆️
fastapi/encoders.py 100% <0%> (ø) ⬆️
tests/test_datetime.py
docs/src/sql_databases/tutorial001.py 100% <0%> (ø)
tests/test_datetime_custom_encoder.py 100% <0%> (ø)
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 92c825b...6f7c4cd. Read the comment docs.

@kabirkhan kabirkhan changed the title Add Open API prefix route Add Open API prefix route - correct docs behind reverse proxy Feb 10, 2019
@kabirkhan
Copy link
Contributor Author

@kabirkhan kabirkhan commented Feb 10, 2019

Fix for #15

@kabirkhan
Copy link
Contributor Author

@kabirkhan kabirkhan commented Feb 13, 2019

@tiangolo would you mind taking a look at this?
I'm happy to add documentation once you review the approach.
I'd love to use the official pypi package rather than my own Github version for a production service at work.
Thanks

@tiangolo tiangolo merged commit 0ea0d0e into tiangolo:master Feb 14, 2019
8 checks passed
8 checks passed
@netlify
Header rules - fastapi No header rules processed
Details
@netlify
Pages changed - fastapi 53 new files uploaded
Details
@netlify
Redirect rules - fastapi No redirect rules processed
Details
@netlify
Mixed content - fastapi No mixed content detected
Details
@codecov
codecov/patch 100% of diff hit (target 100%)
Details
@codecov
codecov/project 100% (+0%) compared to 92c825b
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@netlify
netlify/fastapi/deploy-preview Deploy preview ready!
Details
@tiangolo
Copy link
Owner

@tiangolo tiangolo commented Feb 14, 2019

Great job @kabirkhan ! Thanks a lot. 🎉 🌮

I added tests and docs on top of your work.

for a production service at work

Are you using it at MS? That's very cool!

tiangolo added a commit that referenced this pull request Feb 14, 2019
@tiangolo
Copy link
Owner

@tiangolo tiangolo commented Feb 14, 2019

I just released 0.4.0 including this PR. You should be able to use it now. 🚀

Also, the relevant docs are here: https://fastapi.tiangolo.com/tutorial/sub-applications-proxy/

@kabirkhan
Copy link
Contributor Author

@kabirkhan kabirkhan commented Feb 14, 2019

@tiangolo yep I'm actually planning to use it for all of my team's ML services at Microsoft. Some of them are getting integrated into the core Windows product and some Office products.

Great job with the documentation too!

@tiangolo
Copy link
Owner

@tiangolo tiangolo commented Feb 17, 2019

That's awesome! 🌮 🍰 🚀

May I quote it (that you at MS are using FastAPI)?

@kabirkhan
Copy link
Contributor Author

@kabirkhan kabirkhan commented Feb 17, 2019

Yeah, go for it!

@sm-Fifteen
Copy link
Contributor

@sm-Fifteen sm-Fifteen commented Dec 6, 2019

I know this is an old issue/PR, but shouldn't root_path be used instead of adding a custom parameter to the application? root_path is already the intended mechanism for routing under a mounted subapplication and what Uvicorn and the ASGI spec in general uses to tell the root application what its URL prefix is (it is ignored when routing, but must be taken into account when reconstructing URLs). FastAPI currently does not acknowledge root_path in its API documentation, only openapi_prefix, which means the routes specified by the API doc and the ones that are actually understood by the application will not match unless the value of root_path is also passed as openapi_prefix when creating the FastAPI application.

@luckydonald
Copy link

@luckydonald luckydonald commented Jan 6, 2020

Huh, sounds about right. @sm-Fifteen you may open a new issue for that.

@tiangolo
Copy link
Owner

@tiangolo tiangolo commented Jun 11, 2020

Yeap @sm-Fifteen, I just merged #1199 addressing root_path 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants