Update from task cef5c1dc-60ce-4986-87db-30725a46661f - #135
Conversation
Key features implemented: - Updated .gitignore to include more comprehensive ignore patterns for temp files, compressed archives, compiled files, and IDE-specific files - Fixed potential illegal raise in ai/retry.py by replacing unreachable raise statement with proper error handling - Addressed potentially uninitialized local variable in services/.../v1/routes.py by ensuring variable assignment before use - Fixed writable file handle closure without error handling in Go files by adding proper defer and error checking - Resolved file not always closed warnings in Python files by implementing proper context managers and exception handling - Corrected module import duplication issues in test files by standardizing import styles - Improved file operation safety across multiple services by adding try/finally blocks and proper resource management - Enhanced error handling in file operations within gRPC and API route handlers The changes significantly improve code safety by addressing resource leaks, uninitialized variables, and improper error handling patterns identified by CodeQL, while maintaining existing functionality.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors file handling across several Python files to use context managers, improves error handling during file closing in Go, cleans up the .gitignore file, and introduces chunk-based request body parsing in the upload routes. The review feedback highlights a potential division-by-zero error and negative ETA calculations in the gRPC upload progress stream, and suggests a more robust, idiomatic approach to parsing the request body in the API routes instead of relying on fragile locals() checks.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| total_chunks = session.get('total_chunks', 1) | ||
|
|
||
| bytes_transferred = chunks_received * chunk_size | ||
| percent_complete = (chunks_received / total_chunks) * 100 |
There was a problem hiding this comment.
If total_chunks is 0 (which can happen if total_size is 0), this will raise a ZeroDivisionError. Guard against division by zero by checking if total_chunks > 0.
| percent_complete = (chunks_received / total_chunks) * 100 | |
| percent_complete = (chunks_received / total_chunks) * 100 if total_chunks > 0 else 100.0 |
| body = await request.body() | ||
| import json | ||
| data = json.loads(body.decode('utf-8')) | ||
| chunk_index_val = data.get("chunk_index") if data else None |
There was a problem hiding this comment.
Checking locals() or conditionally parsing the request body in multiple places is fragile and unidiomatic. Instead of checking 'data' not in locals(), we can safely retrieve data from locals() and parse the body only if it hasn't been parsed yet.
| body = await request.body() | |
| import json | |
| data = json.loads(body.decode('utf-8')) | |
| chunk_index_val = data.get("chunk_index") if data else None | |
| data = locals().get('data') | |
| if data is None: | |
| body = await request.body() | |
| import json | |
| data = json.loads(body.decode('utf-8')) if body else {} | |
| chunk_index_val = data.get("chunk_index") if data else None |
| if 'data' not in locals(): | ||
| body = await request.body() | ||
| import json | ||
| data = json.loads(body.decode('utf-8')) | ||
| chunk_hash_val = data.get("chunk_hash") if data else None |
There was a problem hiding this comment.
Checking 'data' not in locals() is fragile and unidiomatic. We can safely retrieve data from locals() and parse the body only if it hasn't been parsed yet.
| if 'data' not in locals(): | |
| body = await request.body() | |
| import json | |
| data = json.loads(body.decode('utf-8')) | |
| chunk_hash_val = data.get("chunk_hash") if data else None | |
| data = locals().get('data') | |
| if data is None: | |
| body = await request.body() | |
| import json | |
| data = json.loads(body.decode('utf-8')) if body else {} | |
| chunk_hash_val = data.get("chunk_hash") if data else None |
| chunk_size = session.get('chunk_size', 1024 * 1024) | ||
| total_chunks = session.get('total_chunks', 1) | ||
|
|
||
| bytes_transferred = chunks_received * chunk_size |
There was a problem hiding this comment.
If the last chunk is smaller than chunk_size, chunks_received * chunk_size can exceed total_size, leading to a negative remaining_bytes and negative eta_seconds. Cap bytes_transferred at total_size.
| bytes_transferred = chunks_received * chunk_size | |
| bytes_transferred = min(chunks_received * chunk_size, total_size) |
|
@qwen-intl fixed all |
2 participants
This PR was created by qwen-chat coder for task cef5c1dc-60ce-4986-87db-30725a46661f.