-
Notifications
You must be signed in to change notification settings - Fork 0
Wire household creation on registration and fix datetime handling #6
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
Changes from all commits
23941ef
c2de597
f4d1730
d72d6b7
8719c64
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,9 @@ | |
|
|
||
| DATABASE_URL = settings.database_url.get_secret_value() | ||
|
|
||
| engine = create_async_engine(DATABASE_URL, echo=settings.environment == "development") | ||
| # pool_pre_ping=True tests each connection before use and reconnects if Neon | ||
| # closed it due to inactivity (Neon drops idle connections after ~5 minutes). | ||
| engine = create_async_engine(DATABASE_URL, echo=settings.environment == "development", pool_pre_ping=True) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Smart addition for Neon!
The performance overhead is minimal (microseconds), and it prevents cryptic "connection closed" errors in production. Good operational awareness — this shows you're thinking about how the code will behave in a real deployment environment, not just on your dev machine. |
||
|
|
||
| # keeps objects usable after commit (relevant for async sessions) | ||
| _session_factory = async_sessionmaker(engine, expire_on_commit=False) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,15 @@ | ||
| from datetime import UTC, datetime | ||
|
|
||
| import sqlalchemy as sa | ||
| from sqlmodel import Field, SQLModel | ||
|
|
||
|
|
||
| class Household(SQLModel, table=True): | ||
| __tablename__: str = "households" | ||
|
|
||
| id: int | None = Field(default=None, primary_key=True) | ||
| name: str | ||
| created_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) | ||
| created_at: datetime = Field( | ||
| default_factory=lambda: datetime.now(UTC), | ||
| sa_column=sa.Column(sa.DateTime(timezone=True), nullable=False), | ||
| ) | ||
|
Comment on lines
+12
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same timezone-aware improvement here — great consistency across models. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import uuid | ||
| from datetime import UTC, datetime | ||
|
|
||
| import sqlalchemy as sa | ||
| from fastapi_users import schemas | ||
| from sqlmodel import Field, SQLModel | ||
|
|
||
|
|
@@ -18,11 +19,13 @@ class User(SQLModel, table=True): | |
| is_active: bool = Field(default=True) | ||
| is_superuser: bool = Field(default=False) | ||
| is_verified: bool = Field(default=False) | ||
| # Nullable until the on_after_register hook creates and assigns a household. | ||
| household_id: int | None = Field( | ||
| default=None, foreign_key="household.id", index=True | ||
| default=None, foreign_key="households.id", index=True | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Excellent catch! The foreign key must reference the table name ( Good attention to detail. |
||
| ) | ||
| created_at: datetime = Field( | ||
| default_factory=lambda: datetime.now(UTC), | ||
| sa_column=sa.Column(sa.DateTime(timezone=True), nullable=False), | ||
| ) | ||
|
Comment on lines
+25
to
28
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Excellent improvement! Using Without With This is a best practice worth carrying forward to any other datetime fields you add. |
||
| created_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) | ||
|
|
||
|
|
||
| class UserRead(schemas.BaseUser[uuid.UUID]): | ||
|
|
||
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.
Great comment! This explains a non-obvious behavior (dependency caching) that could confuse someone reading the code.
This is exactly the kind of comment that adds value — it documents a framework behavior that isn't immediately obvious from the code itself.