-
Notifications
You must be signed in to change notification settings - Fork 56
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
Develop #24
base: main
Are you sure you want to change the base?
Develop #24
Conversation
Changed the database file from `example.db` to `movies.db` to reflect the application's focus on movies. This ensures the database aligns better with the application's context and purpose.
Pass the DATABASE_URL to the get_db_contextmanager function to ensure the correct database connection is established. This change improves configurability and aligns the code with the updated database session management approach.
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 job on implementing the task requirements! 🎉 The code is well-structured and meets the core functionality needed. There are a couple of minor issues to address: update the import paths in src/routes/movies.py
to include src.
, and remove the unnecessary DATABASE_URL
parameter from the get_db_contextmanager
call. These changes will ensure consistency and correctness in your project. Keep up the excellent work! 🚀
✨ Please rate this review in the Platform's notification center. I will be grateful for your feedback! ✨
@@ -65,7 +66,7 @@ def seed(self): | |||
|
|||
def main(): | |||
settings = get_settings() | |||
with get_db_contextmanager() as db_session: | |||
with get_db_contextmanager(DATABASE_URL) as db_session: |
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.
The get_db_contextmanager
function is being called with DATABASE_URL
, but according to its definition, it does not accept any parameters. You should remove DATABASE_URL
from the call to fix this issue.
@@ -3,8 +3,41 @@ | |||
|
|||
from database import get_db, MovieModel |
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.
The import statement for get_db
and MovieModel
should be prefixed with src.
to match the project structure. Change from database import get_db, MovieModel
to from src.database import get_db, MovieModel
.
No description provided.