Skip to content

Fix SDK data handling, error reporting, and transfers - #575

Open
mikhailbovt wants to merge 1 commit into
roboflow:mainfrom
mikhailbovt:fix/sdk-data-handling-and-transfers
Open

mikhailbovt wants to merge 1 commit into
roboflow:mainfrom
mikhailbovt:fix/sdk-data-handling-and-transfers

Conversation

@mikhailbovt

Copy link
Copy Markdown

What does this PR do?

Fixes incorrect data handling and false success reporting across SDK operations, and reduces memory use and write overhead during transfers.

  • Preserve detection coordinates when no resize occurs and rescale boxes using the actual inference image dimensions.
  • Resolve COCO image references relative to their annotation file, keeping identically named images in different splits separate and rejecting ambiguous matches.
  • Propagate dataset and model upload failures, report actual upload counts, and return structured CLI errors with the documented exit codes. CLI model uploads no longer prompt for packaging overrides.
  • Reuse dataset downloads only when their completion metadata matches the requested dataset, format, and API endpoint; protect unrelated non-empty directories.
  • Reject failed or truncated weight downloads and replace existing weights only after a successful download.
  • Refresh Vision Events endpoints after region or environment changes.
  • Raise on failed training, including when stale results are present.
  • Stream local video uploads from disk instead of reading the entire file into memory.
  • Buffer dataset, weight, and search-export downloads in 64 KiB chunks with byte-based progress and response-length validation.

Type of Change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Testing

  • I have tested this change locally
  • I have added/updated tests for this change

Test details:
Added 40 test methods, including parameterized subtests, and updated 8 existing test methods across 12 test files. Three test modules are new.

Added regression coverage:

Test files What the new tests verify
tests/models/test_inference.py (new) Video uploads pass an open file stream and close it on success, HTTP rejection, or connection failure; hosted URLs skip file upload. Weight downloads preserve existing files on HTTP errors, interrupted streams, and truncated plain/gzip responses, clean up temporary files, and replace weights with the exact response bytes on success.
tests/test_model_upload.py (new) Version and workspace model uploads propagate failures during upload preparation and the subsequent PUT, send the expected archive bytes, and close the upload file on success and failure.
tests/util/test_general.py (new) Downloads preserve bytes with valid, missing, invalid, or negative Content-Length; handle empty responses/chunks; count progress in bytes; and handle gzip and chunked responses without using an incorrect progress total.
tests/models/test_object_detection.py Small, equal-size, large, portrait, and landscape inputs produce the expected coordinates; tests decode the actual request image to check its dimensions and verify unchanged coordinates when resizing is disabled.
tests/util/test_folderparser.py Identical filenames across splits map to the correct annotations; relative paths and Windows separators work; unique basename/stem fallbacks remain supported; ambiguous matches raise; RLE segmentation data is preserved.
tests/test_version.py Completed downloads are reused; HTTP, ZIP, and YAML failures remain retryable; unrelated directories and mismatched dataset/format/version/API metadata are rejected unless overwrite is explicit. Failed training raises immediately or after polling, including with stale results/models; successful training still returns a model.
tests/test_workspace.py Default per-image uploads still return None; opt-in summaries count uploaded and duplicate images; all images are attempted before aggregate failures are raised; failed ZIP processing raises.
tests/test_vision_events.py An existing workspace follows US/EU, production/staging, and explicit API URL changes while preserving its authorization header.
tests/cli/test_image_handler.py, tests/cli/test_model_handler.py Failed uploads emit JSON errors without success output; HTTP 401/404/500 map to exit codes 2/3/1; missing keys return exit code 2; SDK authentication remains catchable as RuntimeError; packaging mismatches never prompt for input. Tests include CliRunner invocations and real SDK initialization against mocked HTTP responses.

Changes to existing tests:

  • Directory-upload CLI tests now mock the opt-in SDK summary and check the actual upload count instead of counting local image files.
  • The model-upload success test now injects SDK stdout noise to verify clean JSON output and checks that interactive=False is passed.
  • The project-upload failure test now expects the aggregate RoboflowError instead of accepting a silent return.
  • The dataset-download return-value test uses a temporary directory instead of a hard-coded filesystem path.
  • The training CLI options test now mocks and asserts the version lookup, keeping the test independent of external API access.

Validation results:

  • Full unittest discovery on Windows with Python 3.10.21 and 3.11.9: 1,318 tests on each, no failures, one expected skip per run.
  • Slim installation: 51 tests passed. Targeted download compatibility checks also passed with urllib3 1.26.6.
  • Ran the Ruff formatting/linting and mypy commands from the Makefile, plus all applicable configured pre-commit hooks, including Bandit. All passed.
  • Tests used mocked HTTP responses and local loopback servers; no live Roboflow uploads or training were performed. Linux and Python 3.12/3.13 remain for CI.

Local performance measurements:

Compared base commit 408c2fe with efd8687 on Windows / Python 3.11.9 using the actual SDK transfer methods and a loopback HTTP server. Uploaded and downloaded bytes were verified with SHA-256.

Measurement Before After
64 MiB video upload: peak Python allocations measured by tracemalloc 64.18 MiB 0.21 MiB
32 MiB download: median elapsed time across three runs 0.2267 s 0.0292 s
64 MiB video upload: elapsed time, single run 0.0813 s 0.1119 s

The traced allocation peak was approximately 99.7% lower, and the local download was approximately 7.8x faster. The video upload took longer in this single-run comparison; its measured benefit is lower memory use. These are local transfer measurements: tracemalloc excludes native allocations and does not measure total process RSS, and the timings do not establish an end-to-end speedup against the Roboflow service. The benchmark is a manual measurement, separate from the unit test suite.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in hard-to-understand areas
  • My changes generate no new warnings or errors
  • I have updated the documentation accordingly (if applicable)

Additional Context

Failed uploads and training now raise exceptions instead of reporting success. Existing non-empty dataset directories without matching completion metadata require overwrite=True or a new destination; interrupted downloads with matching SDK metadata can be retried.

Successful per-image upload_dataset() calls still return None by default; counts are opt-in through return_summary=True. SDK model packaging remains interactive by default, and API-key authentication failures remain catchable as RuntimeError.

The changelog and affected public method docstrings are updated. No dependencies or CLI commands were added.

Preserve box coordinates when images are not resized, resolve COCO image
references using contextual paths, and propagate upload and training
failures. Refresh Vision Events endpoints after region changes.

Stream video uploads and buffer downloads with byte progress and strict
Content-Length validation. Replace weights only after a successful
download, and distinguish completed dataset caches from interrupted
downloads and unrelated directories.

Keep upload_dataset's default None result and expose counts on request
for the CLI. Reject packaging mismatches without prompting in CLI model
uploads while retaining the SDK's interactive default. Discard SDK logs
without retaining them in memory or failing on Unicode output.

Preserve HTTP status through upload failures and map CLI errors to the
documented authentication and not-found exit codes.

This branch has not been deployed

No deployments
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