Feature/error handling#241
Conversation
…for structured statistics
There was a problem hiding this comment.
Code Review
This pull request introduces client-side error collection and typed statistics to the API Client component as a compatibility shim until native server-side persistence is implemented. It defines new Pydantic models (HarvestError, HarvestErrorType, and HarvestStatistics) and updates the parallel ARC submission logic to capture duplicates and submission failures. Documentation and architecture diagrams are also updated to reflect these changes. The review feedback highlights a potential memory leak in api_client.py where completed tasks are not removed from the task_identifiers dictionary, and suggests using .pop() to allow immediate garbage collection.
…anced_middleware_api into feature/error_handling
…mand in update-apk-dependencies.sh
…anced_middleware_api into feature/error_handling
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the design, specification, and implementation of the Harvest Client within the API Client component. It refactors the ApiClient to orchestrate the harvest lifecycle with parallel ARC submissions, client-side duplicate detection, and structured error collection (HarvestError, HarvestErrorType, HarvestStatistics) as a compatibility shim. Additionally, it updates documentation, Dockerfile dependencies, and helper scripts. The code review feedback identifies an issue in _process_completed_arc_tasks where returning early on a catastrophic error leaves remaining completed tasks unprocessed, potentially causing Python's asyncio to log warnings about unretrieved task exceptions. A refactoring is suggested to consume all task results before returning.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request implements client-side error collection and reporting within the ApiClient harvest lifecycle as a compatibility shim. It introduces typed HarvestStatistics and HarvestError models, updates the HarvestResult schema, and documents the harvest client's design and requirements. Additionally, it updates Docker dependencies, revises the architecture diagram, and marks list_harvests as not implemented pending server-side support. The reviewer recommends updating the unit tests to explicitly assert that result.errors is populated with the correct HarvestError objects during duplicate detection and submission failures.
No description provided.