Conversation
9f35763 to
457b390
Compare
|
+1 |
|
+1 |
457b390 to
5ad2af6
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
…_existing_app ✨ Allow to use an existing app
5ad2af6 to
fce4d85
Compare
|
+1 |
This comment was marked as resolved.
This comment was marked as resolved.
fce4d85 to
14cb9ad
Compare
This comment was marked as resolved.
This comment was marked as resolved.
|
+1 |
This comment was marked as resolved.
This comment was marked as resolved.
14cb9ad to
0961863
Compare
This comment was marked as resolved.
This comment was marked as resolved.
|
Can you please help this feature to go through @tiangolo? it's very annoying to see unnecessary access logs in production servers |
|
+1 I need ECS logging for fastapi run |
26404c4 to
c1974b1
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
|
+1 i really need to change the uvicorn log level because some path like readiness and liveness in k8s are called every few seconds and the exception will "disappear" inside the logs after few minutes. |
|
+1 I also need this as the default logger doesn't have timestamps. When errors occur, it makes troubleshooting much more difficult. There are workarounds, but being able to specify a logging override would be the easiest method. |
|
I too would love to be able to override the logger - I'd like to be able to use the same formatter that's used for my application code. Especially the lack of timestamps for access logs is quite annoying. |
This comment was marked as resolved.
This comment was marked as resolved.
c1974b1 to
98095ce
Compare
98095ce to
74093c6
Compare
|
+1, imo this is what stops this wrapper from being genuinely production-ready. |
|
Following up on my previous comment saying that after some studies I will suggest for production service to avoid You can create the import uvicorn
uvicorn.run(
"app_folder.app_creator:app",
host=<host>, # example: "0.0.0.0"
port=<port>, # example: 8080
workers=<workers>, # example: 3
reload=<reload>, # false in production
log_config=<logging_config dict> # Optional
)
In from fastapi import FastAPI
app = FastAPI(
title="Your API",
# all needed parameters and configurations
)
# Add routers, middleware, exception handlers, etc.
app.include_router(your_router) |
252a50d to
e78f460
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds support for custom logging configuration in FastAPI CLI by allowing users to pass a --log-config option to both fastapi dev and fastapi run commands. The implementation properly falls back to uvicorn's default logging configuration when no custom config is provided.
Key changes:
- Added
--log-configparameter todevandrunCLI commands that accepts a Path to a logging configuration file - Updated
_runfunction to accept and conditionally use custom log config, falling back toget_uvicorn_log_config()when not provided - Added comprehensive test coverage including argument validation and help text verification
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/fastapi_cli/cli.py | Added log_config parameter to _run, dev, and run functions with conditional logic to use custom config or fall back to default |
| tests/test_cli.py | Added test cases verifying --log-config argument is properly passed to uvicorn and help text is correctly displayed |
| tests/assets/log_config.yaml | Added sample logging configuration file for testing purposes with standard Python logging dict config format |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
|
@YuriiMotov Sorry to spam you. Can i ask you to add Feature label like it was done for #160 Thank you |
This comment was marked as resolved.
This comment was marked as resolved.
YuriiMotov
left a comment
There was a problem hiding this comment.
LGTM!
@07pepa, thanks!
As we don't have exist=True parameter, we don't actually need to add the tests/assets/log_config.yaml file.
But it will be needed if we decide to add exist=True.
| Path | None, | ||
| typer.Option( | ||
| help="Logging configuration file. Supported formats: .ini, .json, .yaml. be tried." | ||
| ), |
There was a problem hiding this comment.
We can also specify:
| ), | |
| dir_okay=False, | |
| exists=True, | |
| ), |
(and the same for run command)
sometimes its nice to have your custom logging in fastapi so i added passing it to uvicorn and fallbacking to uvicorn default one if nothing is passed in