Skip to content

Feature/error handling#241

Merged
Zalfsten merged 14 commits into
mainfrom
feature/error_handling
May 27, 2026
Merged

Feature/error handling#241
Zalfsten merged 14 commits into
mainfrom
feature/error_handling

Conversation

@Zalfsten
Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread middleware/api_client/src/middleware/api_client/api_client.py Outdated
@Zalfsten
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread middleware/api_client/src/middleware/api_client/api_client.py Outdated
@Zalfsten
Copy link
Copy Markdown
Contributor Author

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread middleware/api_client/src/middleware/api_client/api_client.py
@Zalfsten Zalfsten merged commit 2f519f4 into main May 27, 2026
14 checks passed
@Zalfsten Zalfsten deleted the feature/error_handling branch May 27, 2026 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant