feat: Clean up sample app and finalize endpoints

[shuangela]

Clean up the order of endpoints, edit comments, and remove print statements. Also noticed some edge cases in code that I fixed.

This PR reorders the endpoints in the FastAPI application to match the other backends. It also removes print statements in preparation for a different PR that introduces proper logging. Finally, it also handles the edge case I noticed in the atlas search endpoint when there are no movies matching the query and removes an error I received when I tried to create multiple movies at once with the batch endpoint.

You will need a Voyage AI API key to test the vector search -- let me know if you need one!

Key Changes

• Changes order of endpoints and renames the search endpoints
• Removes print statements for future logging implementation
• Handles edge cases I found when testing in atlas search
• Add comment explaining why search and vector search are above the other endpoints

Testing

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

[dacharyc]

Thanks for putting this up, @shuangela ! There's a bit more cleanup we could do here - removing unused imports, changing how we're handling things in a couple of places, etc.

Let me know what you think you might have time to handle, and we can scope any of the rest of this for fast follows.

I'd like to take one more look if you make any of the changes I'm suggesting here aside from the import cleanup, so LMK how you want to proceed!

[dacharyc]

You're adding create_error_response here but it looks like it's not used. Are we missing code that was meant to use it, or was this added in error?

Suggested change

	from src.utils.errorHandler import register_error_handlers, create_error_response
	from src.utils.errorHandler import register_error_handlers

[shuangela]

added in error, we only use it in models.py. i will remove it

[dacharyc]

Looks like we're not using this import - maybe it's left over from a prior version of this file?

Suggested change

import traceback

[dacharyc]

Looks like we're not doing anything with this result, so we could omit it.

Suggested change

	result = await embedded_movies_collection.create_search_index(index_definition)
	await embedded_movies_collection.create_search_index(index_definition)

[dacharyc]

It looks like the on_event handler we're using here is deprecated. The docs say we should be using lifespan event handlers instead: https://fastapi.tiangolo.com/advanced/events/#alternative-events-deprecated

I don't know how big of a lift it is to switch to the lifespan event handler. If this isn't a quick fix, I'd say we should scope this for a fast follow given the release timeline. But ideally we would not release a brand new sample app that uses deprecated APIs.

[shuangela]

i will look into this and try to implement it in my upcoming push!

[dacharyc]

Looks like this is an unused import.

Suggested change

	from src.database.mongo_client import db, get_collection, voyage_ai_available
	from src.database.mongo_client import get_collection, voyage_ai_available

[dacharyc]

Looks like these are unused imports. Is this functionality we haven't implemented yet, or functionality that we're not handling here in the router?

Suggested change

	from src.models.models import VectorSearchResult, CreateMovieRequest, Movie, MovieFilter, SuccessResponse, UpdateMovieRequest, SearchMoviesResponse, BatchUpdateRequest, BatchDeleteRequest
	from src.models.models import VectorSearchResult, CreateMovieRequest, Movie, SuccessResponse, UpdateMovieRequest, SearchMoviesResponse

[shuangela]

It seems like these models were added 2 days ago by Jordan. @jordan-smith721 do you have any more information on these imports and if we are good to delete them, or if you added them as planned scaffolding for a future pr?

[jordan-smith721]

I think we're good to delete. I'd assume they were added while I was iterating through some changes but then ended up going a different route. If we need to reimport them with a later PR then we can.

[dacharyc]

I can't comment directly on the line below, but it also looks like we're not using datetime here so we should omit this import.

[dacharyc]

I notice here and throughout this file, we're saying that the APIs return SuccessResponse[Movie], but in many cases, we're returning an ErrorResponse type (i.e. return create_error_response). While this technically works in Python, it's not a good pattern. Looks like the recommended way to handle this for FastAPI is actually to raise an HTTPException so we get the appropriate status code. i.e.:

from fastapi import HTTPException

# Instead of:
return create_error_response(
    message="Invalid movie_id format.",
    code="INVALID_OBJECT_ID",
    details=str(movie_id)
)

# Do this:
raise HTTPException(
    status_code=400,
    detail={
        "message": "Invalid movie_id format.",
        "code": "INVALID_OBJECT_ID",
        "details": str(movie_id)
    }
)

If this is something you could fix up before MVP launch, that would be ideal, but if not, this is definitely something we should scope for a fast follow.

[shuangela]

Hey Dachary,

This is the pattern I initially had (HTTPException), but Taylor recommended changing my uses of httpexception to use create_error_response instead in a previous PR: #5 (comment). Do you think we should change it back, or wait until Taylor comes back to discuss this?

[shuangela]

We discussed this over slack -- this will be a fast follow

[dacharyc]

My IDE's type hints are complaining about this object_id because of type inference - my IDE is expecting type dict[str, str | int] but getting type ObjectId instead.

A way to fix this would be to use Any or more flexible typing for the pipeline - where you declare this pipeline up above in line 989, add this:

pipeline: list[dict[str, Any]] = [

Then the IDE will know that object_id is a valid type here. (If you wanted to make that fix, you'd also need to import Any from typing at the top of the file - you're already importing List so you could add Any with it.

[shuangela]

Ah, gotcha! I don't think I see that on my IDE but i will make that fix

[dacharyc]

LGTM once the merge conflicts are resolved! Thanks for making those tweaks! ✅