Skip to content

feat(s3,utils): use niquests on_upload hook and optimize stream chunking#49

Merged
Andarius merged 6 commits intomasterfrom
feat/s3-upload-progress-and-stream-chunk-cleanup
Feb 26, 2026
Merged

feat(s3,utils): use niquests on_upload hook and optimize stream chunking#49
Andarius merged 6 commits intomasterfrom
feat/s3-upload-progress-and-stream-chunk-cleanup

Conversation

@Andarius
Copy link
Contributor

@Andarius Andarius commented Feb 26, 2026

  • Split get_stream_chunk into get_stream_chunk (bytes, uses bytearray internally) and get_stream_chunk_str, removing 5 redundant helper functions
  • Replace on_chunk_received with niquests on_upload hook in s3_put_object, s3_multipart_upload, and s3_file_upload for fine-grained byte-level upload progress
  • Make S3Session fields (endpoint_url, access_key, secret_key, region) optional, falling back to botocore defaults
  • Widen s3_put_object and upload_part to accept bytes | bytearray

Summary by CodeRabbit

  • New Features

    • S3 session configuration now supports optional credentials for flexible deployment scenarios
    • S3 upload operations expanded to accept additional data types
    • Enhanced upload progress tracking mechanism
  • Tests

    • Comprehensive test coverage added for stream processing with multiple scenarios

- Split `get_stream_chunk` into `get_stream_chunk` (bytes, uses `bytearray` internally) and `get_stream_chunk_str`, removing 5 redundant helper functions
- Replace `on_chunk_received` with niquests `on_upload` hook in `s3_put_object`, `s3_multipart_upload`, and `s3_file_upload` for fine-grained byte-level upload progress
- Make `S3Session` fields (`endpoint_url`, `access_key`, `secret_key`, `region`) optional, falling back to botocore defaults
- Widen `s3_put_object` and `upload_part` to accept `bytes | bytearray`
@coderabbitai
Copy link

coderabbitai bot commented Feb 26, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8463e30 and c918ec1.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (4)
  • tests/s3/test_niquests.py
  • tests/test_utils.py
  • tracktolib/s3/niquests.py
  • tracktolib/utils.py

Walkthrough

The pull request refactors the upload callback mechanism from on_chunk_received to on_upload (accepting PreparedRequest), makes S3Session configuration fields optional with defaults, and separates stream chunking into distinct paths for bytes and strings with a new BytesBuffer utility class.

Changes

Cohort / File(s) Summary
Test Suite Updates
tests/s3/test_niquests.py, tests/test_utils.py
Updated S3 test to use new on_upload callback signature and replaced received_size assertions with upload_progress_called flag. Added parametric test_get_stream_chunk_str function with three test scenarios for string chunking.
S3 Upload Module
tracktolib/s3/niquests.py
Introduced OnUpload type alias; made S3Session fields (endpoint_url, access_key, secret_key, region) optional with None defaults; guarded credential setup; extended function signatures to accept `bytes
Stream Utilities
tracktolib/utils.py
Renamed previous generic get_stream_chunk to get_stream_chunk_str for string streams; created new get_stream_chunk for bytes streams using AsyncIterable[bytes]; introduced BytesBuffer class with put, get, get_all methods for efficient bytes accumulation; expanded imports to include collections and io.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/s3-upload-progress-and-stream-chunk-cleanup

Comment @coderabbitai help to get the list of available commands and usage tips.

@Andarius Andarius self-assigned this Feb 26, 2026
Replace bytearray (extend + slice) with a deque-based `BytesBuffer`
adapted from urllib3's `BytesQueueBuffer`. O(1) appends with no copy,
memoryview-based splitting at chunk boundaries. 3-5x faster for
medium/large chunks (64KB-1MB) typical of S3 multipart uploads.
@Andarius Andarius marked this pull request as ready for review February 26, 2026 20:30
@Andarius Andarius merged commit dc7dd7d into master Feb 26, 2026
3 of 4 checks passed
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.

2 participants