GET routes/{route_id}

[Artyom-G]

JIRA ticket link

Ticket Name

Implementation description

• Added a GET routes/{route_id} route

Steps to test

1. Pick an arbitrary route_id (you can select this by testing GET /routes)
2. Test GET /routes/{route_id}
3. Both endpoints require auth (in theory) but this seems to fail, I can work on that but under a different ticket since that seems like an auth issue

What should reviewers focus on?

• I was following the other routes as a guide, I wonder if this is the best way to do it. Look at my TODO comment under the DELETE routes/{route_id}:

    async def delete_route(self, session: AsyncSession, route_id: UUID) -> bool:
        """Delete route by ID"""
        try:
            statement = select(Route).where(Route.route_id == route_id)
            result = await session.execute(statement)
            route = result.scalars().first()

            if not route:
                self.logger.error(f"Route with id {route_id} not found")
                return False

            await session.delete(route)
            await session.commit()

            return True
        except Exception as error:
            self.logger.error(f"Failed to delete route {route_id}: {error!s}")
            await session.rollback()
            # TODO: do we really want to return the raw error
            raise error

• NOTE: This is not my code, I used this as a reference

Checklist

• My PR name is descriptive and in imperative tense
• My commit messages are descriptive and in imperative tense. My commits are atomic and trivial commits are squashed or fixup'd into non-trivial commits
• I have requested a review from the PL, as well as other devs who have background knowledge on this PR or who will be building on top of this PR

[ludavidca]

It's good that you are leaving docstring, but I think that this might be a little too verbose. Just a description, parameter, and returns are fine. Auth should be evident by reading code and raises can be a quick reference to 404 (500 comes default)!

[ludavidca]

Remove To Do, good catch, and I will make sure to address this soon.

[ludavidca]

I think this error handling would raise a 500 Internal Server Error rather than a 404. It would be better to do something like:

raise HTTPException(
status_code=404,
detail=f"Route with id {route_id} not found"
)

[ludavidca]

What does %s mean?

[ludavidca]

Should be good for now! There might be logging that is done at a level above; this just raises an error.