Forwarding capture metadata problems to users on capture creation#303
Merged
Conversation
|
Review changes with Changed Files |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 03098e0. Configure here.
lucaspar
force-pushed
the
lp/sharing-capture-error-info
branch
from
122a616 to
068b1ce
Compare
Member
Author
|
rebased |
The broad except around fget_object was converting every storage failure, including MinIO connection errors, into ValueError. Capture create/update then returned HTTP 400 for infrastructure outages while OpenSearch connection failures correctly returned HTTP 503. Classify storage errors with is_storage_unavailable_error(), raise StorageUnavailableError for unreachable storage, and keep ValueError for missing or user-actionable fetch failures. Handle StorageUnavailableError in capture create/update alongside OpenSearch ConnectionError. Co-authored-by: Lucas Parzianello <lucaspar@users.noreply.github.com>
- Combine nested with statements in test_minio (SIM117) - Merge storage/OpenSearch 503 handlers in capture update (PLR0911) - Avoid direct urllib3 import in test_storage_errors (deptry DEP003)
reconstruct_tree was applying the 5GB download safety buffer when checking space for tiny metadata fetches in tempfile directories, which failed in CI after deploy when free disk dropped below 5GB. Skip the buffer for reconstruction (buffer_bytes=0) and mock disk space in celery email tests that exercise MinIO rather than disk availability.
lucaspar
force-pushed
the
lp/sharing-capture-error-info
branch
from
068b1ce to
f5b6d7c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ValueErrorexception, since they're likely problems with the data, not the systemNote
Medium Risk
Changes HTTP status mapping on capture create/update (400 vs 503) and MinIO/metadata failure paths; behavior is covered by new tests but affects a core ingestion flow.
Overview
Improves capture create/update error handling so users get actionable 4xx responses for bad upload data instead of opaque 5xx failures, while true infrastructure problems still surface as 503.
Digital RF metadata (
read_metadata_by_channel): broadOSErrorfrom missing/corrupt RF or metadata HDF5 is wrapped inValueErrorwith re-upload guidance; corruptmetadata@*.h5afterread_flatdictfails is treated the same way (optional digital metadata withoutdmd_propertiesstill logs and continues).MinIO file tree reconstruction: fetch errors are split via
storage_errors— connectivity/timeouts/service errors raiseStorageUnavailableError(→ 503 on capture endpoints); missing objects and other fetch failures raiseValueErrorwith user-facing messages (→ 400). Temp-dir reconstruction usescheck_disk_space_available(..., buffer_bytes=0)so the 5GB safety buffer does not block small staging dirs.Adds
scripts/diagnose_metadata_files.pyto download capture/file UUIDs from MinIO and report HDF5 health (dmd_properties,metadata@files). Tests cover storage classification, reconstruct_tree behavior, metadataValueError, and capture 503 paths.Reviewed by Cursor Bugbot for commit 122a616. Bugbot is set up for automated code reviews on this repo. Configure here.