-
Notifications
You must be signed in to change notification settings - Fork 164
fix(BA-1998): Resolve VFolder permissions field in Compute session node #5322
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
Conversation
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.
Pull Request Overview
This pull request adds a null check to the VFolder node resolver to prevent potential null reference errors when a VFolder row is not found in the database.
- Adds explicit type casting and null validation for the database query result
- Returns
None
early when no VFolder row is found instead of proceeding with potentially null data
@@ -258,6 +259,9 @@ async def get_node( | |||
query = query.where(sa.and_(cond, VFolderRow.id == uuid.UUID(vfolder_row_id))) | |||
async with graph_ctx.db.begin_readonly_session(db_conn) as db_session: | |||
vfolder_row = await db_session.scalar(query) | |||
vfolder_row = cast(Optional[VFolderRow], vfolder_row) |
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 cast to Optional[VFolderRow] is unnecessary since the database query already returns Optional[VFolderRow]. This cast doesn't provide additional type safety and adds redundant code.
vfolder_row = cast(Optional[VFolderRow], vfolder_row) |
Copilot uses AI. Check for mistakes.
@@ -258,6 +259,9 @@ async def get_node( | |||
query = query.where(sa.and_(cond, VFolderRow.id == uuid.UUID(vfolder_row_id))) | |||
async with graph_ctx.db.begin_readonly_session(db_conn) as db_session: | |||
vfolder_row = await db_session.scalar(query) | |||
vfolder_row = cast(Optional[VFolderRow], vfolder_row) | |||
if vfolder_row is None: | |||
return None |
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 indentation level suggests this null check is only executed within the database session context, but it should be performed after the session regardless of the query path. Consider moving this check outside the conditional block to ensure it covers all code paths.
Copilot uses AI. Check for mistakes.
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.
It works! LGTM
resolves #5321 (BA-1998)
Checklist: (if applicable)
ai.backend.test
docs
directory