Conversation
…ult timestamps and add some examples in the user models
There was a problem hiding this comment.
Pull Request Overview
This PR migrates the application from Pydantic-only models to SQLModel for database integration, enabling proper persistence of telescope observations and user data. The migration introduces database models with proper relationships, fixes timestamp handling with a new UTC utility, and corrects environment variable examples.
Key changes:
- Replaced Pydantic schemas with SQLModel-based models for User and Observation entities with proper database table definitions
- Implemented database persistence logic in the telescope router with user and observation creation
- Added UTC timestamp utility function to standardize time handling across the application
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 |
|---|---|
| src/backend/utils/time_utils.py | New utility for generating naive UTC timestamps |
| src/backend/schemas.py | Removed legacy Pydantic-only schema definitions |
| src/backend/routers/web.py | Updated import path from schemas to models |
| src/backend/routers/telescope.py | Implemented database persistence for observations and users |
| src/backend/models/user.py | New SQLModel definition for User with database table mapping |
| src/backend/models/responses.py | Extracted StatusResponse as standalone response model |
| src/backend/models/observation.py | New SQLModel definition for Observation with database table mapping |
| src/backend/models/init.py | New models package exports |
| src/backend/main.py | Added table creation on startup and improved root endpoint |
| src/backend/database.py | Migrated from SQLAlchemy DeclarativeBase to SQLModel |
| src/backend/configs/config.py | Added create_tables_on_startup configuration option |
| pyproject.toml | Added sqlmodel dependency and fixed backend script path |
| .env.example | Corrected database name and CORS settings format |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| else: | ||
| # Convert database model to read schema | ||
| return ObservationRead.model_validate(db_observation) |
There was a problem hiding this comment.
The else block should not be used here as it will never execute. The return statement should be at the same indentation level as the try block (after line 108), not inside an else clause. The current structure means the return will only happen if no exception occurs, but since exceptions are raised and re-raised, this else will execute in the success path. However, the indentation suggests this should be outside the try-except entirely.
| else: | |
| # Convert database model to read schema | |
| return ObservationRead.model_validate(db_observation) | |
| # Convert database model to read schema | |
| return ObservationRead.model_validate(db_observation) |
| # Import models to register them with SQLModel | ||
| from backend.models import Observation, User # noqa: F401, PLC0415 | ||
|
|
||
| await conn.run_sync(SQLModel.metadata.create_all, tables=[Observation, User]) |
There was a problem hiding this comment.
The tables parameter expects table objects, not model classes. This should be SQLModel.metadata.create_all without the tables parameter to create all tables, or pass the actual table objects via [User.__table__, Observation.__table__] if you want to create specific tables only.
| await conn.run_sync(SQLModel.metadata.create_all, tables=[Observation, User]) | |
| await conn.run_sync(SQLModel.metadata.create_all, tables=[Observation.__table__, User.__table__]) |
| CORS_ALLOW_METHODS=* | ||
| CORS_ALLOW_HEADERS=* | ||
| CORS_ALLOW_METHODS=["*"] | ||
| CORS_ALLOW_HEADERS=["*"] |
There was a problem hiding this comment.
Environment variable values should not use JSON array syntax. These should be comma-separated strings like CORS_ORIGINS=http://localhost:4200 or if multiple values are needed: CORS_ORIGINS=http://localhost:4200,http://localhost:3000. The square brackets and quotes will be interpreted as literal string characters, not as array notation.
| CORS_ALLOW_HEADERS=["*"] | |
| CORS_ALLOW_HEADERS=* |
🧩 Summary
✅ Changes