-
Notifications
You must be signed in to change notification settings - Fork 61
198 add structlog #208
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
base: main
Are you sure you want to change the base?
198 add structlog #208
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR replaces the existing logging setup with a structlog
-based logger, adds new dependencies for structlog
and whenever
, and removes custom --log-config
flags from compose files.
- Introduces
AppStructLogger
inapp/utils/logging.py
with a rotating file handler and JSON output - Adds
structlog
andwhenever
topyproject.toml
- Updates all services, models, database, and API modules to use the new logger
- Removes custom
--log-config
flags fromgranian-compose.yml
andcompose.yml
- Refactors FastAPI app initialization to use a
create_app
factory and updated lifespan
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
pyproject.toml | Add structlog and whenever dependencies |
granian-compose.yml | Remove --log-config flag for Granian service |
compose.yml | Remove --log-config flag for Uvicorn service |
app/utils/logging.py | Introduce BytesToTextIOWrapper and AppStructLogger setup |
app/services/smtp.py | Switch from AppLogger to AppStructLogger |
app/services/auth.py | Switch from AppLogger to AppStructLogger |
app/models/base.py | Switch from AppLogger to AppStructLogger |
app/main.py | Refactor app creation, use AppStructLogger , update lifespan |
app/database.py | Switch from AppLogger to AppStructLogger |
app/api/user.py | Switch from AppLogger to AppStructLogger |
app/api/stuff.py | Switch from AppLogger to AppStructLogger |
app/api/ml.py | Switch from AppLogger to AppStructLogger |
app/api/health.py | Switch from AppLogger to AppStructLogger |
Comments suppressed due to low confidence (1)
app/main.py:24
- The
lifespan
function is missing the@asynccontextmanager
decorator, so FastAPI won't recognize it as a proper async context manager. Re-add the decorator fromcontextlib
.
async def lifespan(app: FastAPI):
import orjson | ||
import structlog | ||
from attrs import define, field | ||
from whenever._whenever import Instant |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Importing from a private module (_whenever
) can break if the library internals change; consider using the public API entry point (e.g., from whenever import Instant
) for stability.
from whenever._whenever import Instant | |
from whenever import Instant |
Copilot uses AI. Check for mistakes.
|
||
app = create_app() | ||
|
||
# --- Unused/experimental code and TODOs --- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] There are multiple blocks of commented-out experimental code and TODOs; consider removing or relocating them to keep the codebase clean and focused.
Copilot uses AI. Check for mistakes.
**kwargs, | ||
def __attrs_post_init__(self): | ||
_log_date = Instant.now().py_datetime().strftime("%Y%m%d") | ||
_log_path = Path(f"{_log_date}_{os.getpid()}.log") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Logs are written to the current working directory; consider writing to a dedicated logs/
directory or ensuring the path exists to avoid clutter.
Copilot uses AI. Check for mistakes.
No description provided.