feat: enhance error logging and add error_handler to queries by ad-claw000 · Pull Request #727 · aperture-data/aperturedb-python

ad-claw000 · 2026-05-21T07:34:12Z

Summary

Enhances the logging mechanism for query errors to avoid unmanageable log sizes when large batches fail. Added an error_handler argument to execute_query, execute_batch_sets, and ParallelQuery generators to allow downstream handling of query errors natively for post-mortem analysis.

Verification

Ran the pytest test/test_Images.py successfully and executed test/test_Parallel.py.

Fixes #172.

Copilot

Pull request overview

This PR enhances error handling in the ApertureDB Python SDK’s batch/parallel query execution path by reducing overly verbose error logs for large failed batches and introducing an error_handler callback to let downstream code capture failed/partial-failure details for post-mortem analysis.

Changes:

Added an optional error_handler callback to CommonLibrary.execute_query and propagated it through ParallelQuery and ParallelQuerySet execution.
Reduced error/partial-error logging verbosity to avoid dumping full queries and large warning payloads.
Added explicit client deletion at the end of ParallelQuery.worker to attempt earlier connection cleanup.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
aperturedb/CommonLibrary.py	Adds `error_handler` support and trims verbose error logging in `execute_query`.
aperturedb/ParallelQuery.py	Plumbs generator-provided `error_handler` into batch execution; adds client cleanup logic in worker threads.
aperturedb/ParallelQuerySet.py	Extends batch-set execution wrapper to accept/pass through `error_handler`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

ad-claw000

All PR review comments have been addressed in the latest commit.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

- Switched from BaseException to Exception in response/error handlers - Re-raised using bare raise to preserve traceback context - Added monkeypatch tests for error_handler scenarios

- restore 'with open' in Sources.py and VideoDataCSV.py to fix UnboundLocalError - restore 'with iolock' in cli/mount_coco.py to avoid deadlock on exception - revert broken DaskManager.py changes which removed 'raise' causing UnboundLocalError - restore dry_run kwarg pass in ParallelQuery.py for DaskManager

Copilot

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

aperturedb/CommonLibrary.py:324

The total-transaction-failure log still interpolates the full query (and full response) into the ERROR message. For large batches this can still generate extremely large log lines, which undermines the stated goal of keeping logs manageable. Consider logging a bounded summary instead (e.g., number of commands, top-level status/info, and optionally a truncated/capped preview), and keep full query/response only behind DEBUG or via the new error_handler.

        # Transaction failed entirely.
        logger.error(
            f"Failed query = {query} with response = {censor_tokens(r)}")
        result = 1

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

…statuses

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

aperturedb/ParallelQuerySet.py:60

execute_batch_sets logs commands_per_query/blobs_per_query before the None-defaults are normalized (they’re set a few lines later), so the info log will often show None rather than the effective values. Consider moving this log statement after the defaults are filled in (or log both the raw input and the computed defaults) to keep the log actionable.

        logger.info("Execute Batch Sets = Batch Size {0}  Comands Per Query {1} Blobs Per Query {2}".format(
            len(query_set), commands_per_query, blobs_per_query))

aperturedb/CommonLibrary.py:324

The total-transaction-failure path still logs the entire query (and full response) at error level. For large batches this can still produce unmanageably large logs, which seems to conflict with the PR goal of limiting error log size. Consider logging a summarized/truncated representation (e.g., number of commands and the first N commands) and rely on error_handler/returned response for full details.

        # Transaction failed entirely.
        logger.error(
            f"Failed query = {query} with response = {censor_tokens(r)}")
        result = 1

- Move logging of commands_per_query and blobs_per_query down so they don't log None - Truncate the failed query in the total transaction failure error log

ad-claw000 · 2026-05-24T11:15:11Z

Addressed the low confidence review comments regarding logging in commit 45d0f95. Moved logging of commands_per_query and blobs_per_query down to correctly log defaults, and truncated the failed query in the total transaction failure error log to prevent unmanageably large logs.

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

… typing

feat: enhance error logging and add error_handler to queries

4aac20c

Copilot AI review requested due to automatic review settings May 21, 2026 07:34

Copilot started reviewing on behalf of ad-claw000 May 21, 2026 07:34 View session

Copilot AI reviewed May 21, 2026

View reviewed changes

Comment thread aperturedb/CommonLibrary.py

Comment thread aperturedb/ParallelQuery.py Outdated

Address Copilot review comments on PR #727

52a5704

ad-claw000 commented May 21, 2026

View reviewed changes

Merge branch 'develop' into fix/issue-172

7c70f2e

Copilot AI review requested due to automatic review settings May 21, 2026 15:00

Copilot started reviewing on behalf of luisremis May 21, 2026 15:01 View session

Copilot AI reviewed May 21, 2026

View reviewed changes

Comment thread aperturedb/CommonLibrary.py Outdated

Comment thread aperturedb/CommonLibrary.py Outdated

ad-claw000 added 3 commits May 21, 2026 15:40

Address review comments for error_handler exception handling and tests

44dc1aa

- Switched from BaseException to Exception in response/error handlers - Re-raised using bare raise to preserve traceback context - Added monkeypatch tests for error_handler scenarios

style: apply pre-commit autopep8 fixes

4ce015a

Copilot AI review requested due to automatic review settings May 21, 2026 16:28

Copilot started reviewing on behalf of ad-claw000 May 21, 2026 16:29 View session

Copilot AI reviewed May 21, 2026

View reviewed changes

Comment thread test/test_ResponseHandler.py Outdated

Comment thread test/test_ResponseHandler.py Outdated

test: correct mocked response formats for error_handler tests

8578978

luisremis assigned ad-claw000 May 21, 2026

merge: resolve conflicts with develop

518f016

Copilot AI review requested due to automatic review settings May 23, 2026 03:46

Copilot started reviewing on behalf of ad-claw000 May 23, 2026 03:47 View session

Copilot AI reviewed May 23, 2026

View reviewed changes

Comment thread aperturedb/CommonLibrary.py Outdated

Comment thread aperturedb/ParallelQuerySet.py Outdated

ad-claw000 added 2 commits May 23, 2026 06:40

fix: address review comments on warn_list and typing

3467faf

fix: initialize default list for commands_per_query and blobs_per_query

f201a47

Copilot AI review requested due to automatic review settings May 23, 2026 11:50

Copilot started reviewing on behalf of ad-claw000 May 23, 2026 11:51 View session

Copilot AI reviewed May 23, 2026

View reviewed changes

Comment thread aperturedb/CommonLibrary.py Outdated

Comment thread aperturedb/ParallelQuerySet.py Outdated

fix: address remaining review comments for error_handler and success_…

1a19100

…statuses

luisremis previously approved these changes May 24, 2026

View reviewed changes

Merge branch 'develop' into fix/issue-172

c0e25bf

Copilot AI review requested due to automatic review settings May 24, 2026 01:34

Copilot started reviewing on behalf of luisremis May 24, 2026 01:34 View session

Copilot AI reviewed May 24, 2026

View reviewed changes

Comment thread aperturedb/ParallelQuerySet.py Outdated

fix: default success_statuses to ParallelQuery.success_statuses

373d387

ad-claw000 dismissed luisremis’s stale review via 373d387 May 24, 2026 02:33

fix: address low confidence review comments regarding logging

45d0f95

- Move logging of commands_per_query and blobs_per_query down so they don't log None - Truncate the failed query in the total transaction failure error log

Copilot AI review requested due to automatic review settings May 24, 2026 11:15

Copilot started reviewing on behalf of ad-claw000 May 24, 2026 11:15 View session

Copilot AI reviewed May 24, 2026

View reviewed changes

Comment thread aperturedb/CommonLibrary.py Outdated

Comment thread aperturedb/ParallelQuerySet.py Outdated

Comment thread aperturedb/ParallelQuerySet.py Outdated

fix: address review comments regarding f-string formatting, typo, and…

7b66be2

… typing

luisremis approved these changes May 25, 2026

View reviewed changes

luisremis merged commit 9a096a0 into develop May 25, 2026
3 checks passed

luisremis deleted the fix/issue-172 branch June 11, 2026 17:27

Uh oh!

Conversation

ad-claw000 commented May 21, 2026

Summary

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

ad-claw000 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

ad-claw000 commented May 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants