feat: crud functionality and aggregations for python backend #5

shuangela · 2025-10-24T19:17:24Z

Basic crud functionality and aggregation pipelines for python backend

This PR introduces basic CRUD functionality and aggregation pipelines for the FastAPI application. It sets up find, insert, delete, and find and delete operations. It also adds aggregations for movies by year, most recent comments (joining the movies collection with the comments collection), and by director.

Key Changes

Added CRUD functionality
Added three aggregation pipelines

Testing

Verified all endpoints locally via FastAPI docs (/docs)
Confirmed database connection and data persistence
Checked error responses for validation and connection issues

cbullinger

a couple comments/questions. nice job!

cbullinger · 2025-10-24T21:33:43Z

server/python/src/routers/movies.py

+@router.get("/{id}", response_model=SuccessResponse[Movie])
+async def get_movie_by_id(id: str):
+    # Validate ObjectId format
+    object_id = ObjectId(id)


should we wrap this in a try-except? i.e. what happens if id isn't valid?

cbullinger · 2025-10-24T21:35:07Z

server/python/src/routers/movies.py

+    # Use findOne() to get a single document by _id
+    movie = await db.movies.find_one({"_id": object_id})

+    movie["_id"] = str(movie["_id"]) # Convert ObjectId to string


same - what happens if movie is None?

cbullinger · 2025-10-24T21:38:11Z

server/python/src/routers/movies.py

+    print(f"Database name: {db.name if hasattr(db, 'name') else 'unknown'}")
+    print(f"Collection name: movies")
+
+    # For motor (async MongoDB driver), we need to await the aggregate call


I thought we weren't using Motor (instead using PyMongo's native async)

Suggested change

# For motor (async MongoDB driver), we need to await the aggregate call

# For async PyMongo driver, we need to await the aggregate call

Correct, we aren't using motor. Just async within the PyMongo driver.

i am not, i think copilot added this comment for some reason despite not using motor 😓

cbullinger · 2025-10-24T21:42:27Z

server/python/src/routers/movies.py

+
+    # For motor (async MongoDB driver), we need to await the aggregate call
+    cursor = await db.movies.aggregate(pipeline)
+    results = await cursor.to_list(length=None)  # Convert cursor to list


do we want to point out why we're using to_list() for aggregations vs. async for for find queries (what you did in lines 105-108)

cbullinger · 2025-10-24T21:44:17Z

server/python/src/routers/movies.py

+    cursor = await db.movies.aggregate(pipeline)
+    results = await cursor.to_list(length=None)  # Convert cursor to list
+
+    print(f"Aggregation returned {len(results)} results")  # Debug logging


is there a ticket to add proper logging?

I don't believe there's an official logging ticket, this was just my logging for my own testing purposes locally. I can remove it if that makes the code cleaner.

tmcneil-mdb

Great job!
These are some minor changes. Mostly to keep the code similar and adding in validation. I didn't get to the end of the file. I will get to find and delete on Monday.

I havent written an aggregation yet, so I might leave those for now. I will ping you, if I get to them.

tmcneil-mdb · 2025-10-24T22:59:32Z

server/python/src/routers/movies.py

+    object_id = ObjectId(id)
+
+    # Use findOne() to get a single document by _id
+    movie = await db.movies.find_one({"_id": object_id})


To grab the db, I added a function called get_collection from mongo_client.py file to make unit testing easier later. I am calling the db like this in the rest of the functions:

movies_collection = get_collection("movies")

tmcneil-mdb · 2025-10-24T23:04:11Z

server/python/src/routers/movies.py

+        genre (str): The genre of the movie.
+        year (int): The year the movie was released.
+        min_rating (float): The minimum IMDB rating.
+        max_rating (float): The maximum IMDB rating.


The request body for this the CreateMovieRequest object.

tmcneil-mdb · 2025-10-24T23:09:15Z

server/python/src/routers/movies.py

+    result = await db.movies.insert_one(movie_data)
+
+    # Retrieve the created document to return complete data
+    created_movie = await db.movies.find_one({"_id": result.inserted_id})


We need to verify that the document was created before querying it. A check that result is acknowledged.

tmcneil-mdb · 2025-10-24T23:16:43Z

server/python/src/routers/movies.py

+        SuccessResponse[Movie]: A response object containing the created movie data.
+"""
+
+@router.post("/", response_model=SuccessResponse[CreateMovieRequest], status_code=201)


Should be:
response_model=SuccessResponse[Movie]

We are returning the movie

tmcneil-mdb · 2025-10-24T23:29:58Z

server/python/src/routers/movies.py

+
+@router.delete("/{id}", response_model=SuccessResponse[dict])
+async def delete_movie_by_id(id: str):
+    object_id = ObjectId(id)


Wrap in a try/catch. Id might not be valid.

tmcneil-mdb · 2025-10-24T23:32:17Z

server/python/src/routers/movies.py

+    result = await db.movies.delete_one({"_id": object_id})
+
+    if result.deleted_count == 0:
+        raise HTTPException(status_code=404, detail="Movie not found")


Lets use the create_error_response() to keep the errors consistent.

tmcneil-mdb · 2025-10-24T23:32:43Z

server/python/src/routers/movies.py

+    object_id = ObjectId(id)
+
+    # Use deleteOne() to remove a single document
+    result = await db.movies.delete_one({"_id": object_id})


Same comment as above about accessing the db.

tmcneil-mdb · 2025-10-27T18:26:21Z

server/python/src/routers/movies.py

+
+@router.delete("/{id}/find-and-delete", response_model=SuccessResponse[Movie])
+async def find_and_delete_movie(id: str):
+    object_id = ObjectId(id)


Wrap in try /except

tmcneil-mdb · 2025-10-27T18:27:18Z

server/python/src/routers/movies.py

+    deleted_movie = await db.movies.find_one_and_delete({"_id": object_id})
+
+    if deleted_movie is None:
+        raise HTTPException(status_code=404, detail="Movie not found")


convert to our standard error response

tmcneil-mdb · 2025-10-27T19:19:10Z

server/python/src/routers/movies.py

+        SuccessResponse[List[dict]]: A response object containing aggregated genre statistics.
+"""
+
+@router.get("/aggregate/by-genre", response_model=SuccessResponse[List[dict]])


Not sure if we are doing by genre?

Either way the endpoint should be /api/movies/reportingByGenre

I'm not against using /aggregate. I think that would look nicer, but its a change we all have to agree on.

good point, i will remove

tmcneil-mdb · 2025-10-27T19:20:19Z

server/python/src/routers/movies.py

+        }
+    ]
+
+    # Execute the aggregation


removed this code

tmcneil-mdb · 2025-10-27T19:21:53Z

server/python/src/routers/movies.py

+        SuccessResponse[List[dict]]: A response object containing movies with their most recent comments.
+"""
+
+@router.get("/aggregate/recent-commented", response_model=SuccessResponse[List[dict]])


same as above.
I think the endpoint is /api/movies/reportingByYear

tmcneil-mdb · 2025-10-27T19:28:17Z

server/python/src/routers/movies.py

+            object_id = ObjectId(movie_id)
+            pipeline[0]["$match"]["_id"] = object_id
+        except Exception:
+            raise HTTPException(status_code=400, detail="Invalid movie_id format")


Standardize error response.

tmcneil-mdb · 2025-10-27T19:28:48Z

server/python/src/routers/movies.py

+    ])
+
+    # Execute the aggregation
+    results = await execute_aggregation(pipeline)


try / except.

tmcneil-mdb · 2025-10-27T19:33:40Z

server/python/src/routers/movies.py

+            "$sort": {"mostRecentCommentDate": -1}
+        },
+        {
+            "$limit": 50 if movie_id else 20


Why not just use limit? You defined it earlier.

good point, i think this is some copilot weirdness i should've caught! fixing

tmcneil-mdb · 2025-10-27T19:34:21Z

server/python/src/routers/movies.py

+        SuccessResponse[List[dict]]: A response object containing yearly movie statistics.
+"""
+
+@router.get("/aggregate/by-year", response_model=SuccessResponse[List[dict]])


/api/movies/reportingByYear

tmcneil-mdb · 2025-10-27T19:36:01Z

server/python/src/routers/movies.py

+    ]
+
+    # Execute the aggregation
+    results = await execute_aggregation(pipeline)


try/ except

tmcneil-mdb · 2025-10-27T19:39:12Z

server/python/src/routers/movies.py

+    ]
+
+    # Execute the aggregation
+    results = await execute_aggregation(pipeline)


try / except

tmcneil-mdb · 2025-10-27T19:39:52Z

server/python/src/routers/movies.py

+        SuccessResponse[List[dict]]: A response object containing director statistics.
+"""
+
+@router.get("/aggregate/directors", response_model=SuccessResponse[List[dict]])


/api/movies/reportingByDirector

add new methods

5ba4f9e

shuangela changed the title ~~Crud and Aggregations for Python~~ feat: crud functionality and aggregations for python backend Oct 24, 2025

shuangela added 2 commits October 24, 2025 15:20

remove comment line

1731299

add comment for vector search

303fecc

shuangela requested review from cbullinger, jordan-smith721 and tmcneil-mdb October 24, 2025 20:00

cbullinger requested changes Oct 24, 2025

View reviewed changes

tmcneil-mdb requested changes Oct 24, 2025

View reviewed changes

tmcneil-mdb reviewed Oct 27, 2025

View reviewed changes

shuangela added 3 commits October 27, 2025 15:40

feedback

e6942b2

pr feedback

aa1db24

remove unneeded imports

0362be7

shuangela requested review from cbullinger and tmcneil-mdb October 27, 2025 20:42

	# For motor (async MongoDB driver), we need to await the aggregate call
	# For async PyMongo driver, we need to await the aggregate call

Uh oh!

feat: crud functionality and aggregations for python backend #5

Are you sure you want to change the base?

feat: crud functionality and aggregations for python backend #5

Conversation

shuangela commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cbullinger left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmcneil-mdb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmcneil-mdb Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmcneil-mdb Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

shuangela commented Oct 24, 2025 •

edited

Loading

tmcneil-mdb Oct 27, 2025 •

edited

Loading

tmcneil-mdb Oct 27, 2025 •

edited

Loading