[Serve] Expose FastAPI docs path #32863
Conversation
Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>
Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>
Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>
Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>
| @@ -51,6 +51,7 @@ def __init__( | |||
| self.deploy_obj_ref = deploy_obj_ref | |||
| self.app_msg = "" | |||
| self.route_prefix = None | |||
| self.fastapi_docs_path = None | |||
There was a problem hiding this comment.
This should not be tied directly to fastapi, we may support other docs links in the future (e.g., if there is a different HTTP wrapper or users provide their own).
Let's call it docs_path everywhere except in the fastapi-specific code.
Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>
| assert num_route_prefixes <= 1, ( | ||
| f"Found multiple route prefix from application {self.name}," | ||
| " Please specify only one route prefix for the application " | ||
| "to avoid this issue." | ||
| ) | ||
| assert num_fastapi_deployments <= 1, ( | ||
| f"Found multiple FastAPI deployments from application {self.name}. " | ||
| "Please only use one FastAPI deployment in your application to avoid " | ||
| "this issue." | ||
| ) |
There was a problem hiding this comment.
please add test cases for these! we should also not use assert for any user-facing error messages (it's written like it might be one)
There was a problem hiding this comment.
Added! I also changed the error message to be more generic in case we decide to expand usage of docs_path in the future.
Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>
Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>
| "to avoid this issue." | ||
| ) | ||
|
|
||
| if "docs_path" in deploy_param and deploy_param["docs_path"] is not None: |
There was a problem hiding this comment.
| if "docs_path" in deploy_param and deploy_param["docs_path"] is not None: | |
| if deploy_param.get("docs_path") is not None: |
Since this is not yet a user-facing variable, I was not planning on adding docs yet. Is there some part of this PR that you think we should add to the docs? When I add the metadata REST endpoint, this will be one of the metadata fields so it will be documented there; also, in the future as Ed suggested we may support other docs links that users themselves provide, and we would probably document that as well. WDYT? |
Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>
Signed-off-by: Cindy Zhang <cindyzyx9@gmail.com>
|
@zcin @shrekris-anyscale I don't think it's worth adding any public-facing documentation for this at the moment. |
|
Link check failing on master & unrelated. |
For FastAPI integrated applications, we want to expose the Open api docs path. Signed-off-by: Jack He <jackhe2345@gmail.com>
For FastAPI integrated applications, we want to expose the Open api docs path. Signed-off-by: Edward Oakes <ed.nmi.oakes@gmail.com>
For FastAPI integrated applications, we want to expose the Open api docs path.
For FastAPI integrated applications, we want to expose the Open api docs path. Signed-off-by: elliottower <elliot@elliottower.com>
For FastAPI integrated applications, we want to expose the Open api docs path. Signed-off-by: Jack He <jackhe2345@gmail.com>
Why are these changes needed?
For FastAPI integrated applications, we want to expose the Open api docs path.
Related issue number
#32878
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.