fps
fps copied to clipboard
Show API summary
See #55
Now looks like this. Websockets are listed at the bottom with a red background for the paths (warning for some potential security issue).

Thanks @davidbrochart ! I'll have a look
@davidbrochart what about making rich an optional dependency with conditional import and optional display of the summary?
We discussed that with @SylvainCorlay, but the point of this PR is making it mandatory to display the API, so that you don't allow e.g. websockets by mistake.
If depending on rich is an issue, we could also show the summary without it, but it probably wouldn't be as beautiful :smile:
On the other hand, I think we could make FPS a Rich/Textual application, instead of having a stream of log messages.
Hey @davidbrochart sorry for the delay.. I don't see the point of displaying the API in the console at server start-up except for development when you need to check quickly if everything is correctly implemented. It's convenient but it also doesn't substitute to unit/integration tests (which is the correct way to do it IMHO) at plugins and app levels to ensure code quality and features fully operational.
I would prefer to make this a dev extra dependency, maybe also move this to fps-uvicorn plugin since the CLI is there and I'm not sure this makes sense for fps itself to print console outputs.
On the other hand, I think we could make FPS a Rich/Textual application, instead of having a stream of log messages.
I don't really get it, could you please expand a bit more?
I support @adriendelsalle on this one. The goal is to have a plugin approach to be able to build the lighter server possible. So everything should be optional including the way we want to display output.
For example log message are really important to be kept as such in deployed scenarios where admin sys usually aggregate them in monitoring third-party service.
I don't see the point of displaying the API in the console at server start-up except for development when you need to check quickly if everything is correctly implemented.
It can also be helpful to ensure there is no security risk, especially if you want to check if the websocket endpoints are served.
On the other hand, I think we could make FPS a Rich/Textual application, instead of having a stream of log messages.
I don't really get it, could you please expand a bit more?
FPS could be have terminal UI, show a dashboard etc.
I support @adriendelsalle on this one. The goal is to have a plugin approach to be able to build the lighter server possible. So everything should be optional including the way we want to display output.
I think FPS being a TUI doesn't break the plugin approach, and it can still be lightweight.
For example log message are really important to be kept as such in deployed scenarios where admin sys usually aggregate them in monitoring third-party service.
We could also dump logs to a file.
The main point here is separation of concern. FPS is (and should focus) on being a pluggable server. Any kind on UI should be a plugin or a separate beast. So why not placing this in a plugin. Indeed the current PR is not breaking it. But we should keep leverage the plugin system every time we can otherwise we will create yet another mammoth out of laziness.
I agree with that. This PR needs access to some of FPS's internals, so I guess we should provide an API to expose them.
This last commit implements (the beginning of) a dashboard, as an FPS plugin. It currently only shows the API summary.
The approach here is to keep FPS as an application that logs to stdout/stderr, and fps-dashboard extracts information from these streams. The log's format will need to be specified.
fps-dashboard executes in a different process than fps-uvicorn.
Actually I'm realizing that the dashboard doesn't need to be an FPS plugin. Maybe it should live in the jupyverse repository, since it will probably be specific to Jupyter.
This PR now only consists of adding a show_endpoints option that logs the HTTP and WebSocket entpoints.
The jupyverse dashboard will consume this information.
@adriendelsalle OK to merge?
Kindly pinging @adriendelsalle.