Skip to content

feat(boxsdk): Replace requests-toolbelt with streaming MultipartStream encoder - #1606

Merged
congminh1254 merged 8 commits into
combined-sdkfrom
remove-requests-toolbelt-legacy
Oct 6, 2026
Merged

congminh1254 merged 8 commits into
combined-sdkfrom
remove-requests-toolbelt-legacy

Conversation

@congminh1254

@congminh1254 congminh1254 commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

Summary

  • back the legacy boxsdk.util.multipart_stream.MultipartStream with the streaming encoder from box_sdk_gen.networking.multipart_stream, which already replaced MultipartEncoder in box_sdk_gen (box/box-codegen#994, released in 4.17.0)
  • keep the MultipartStream(data, files) constructor and data-before-files ordering; accept str, bytes and stream values, and (file_name, value) / (file_name, value, content_type) tuples (the avatar upload uses the 3-tuple)
  • remove requests-toolbelt from install_requires; it was only used by this class
  • update the streaming upload note in docs/boxsdk/usage/files.md

⚠️ Depends on box/box-codegen#995

This class reuses the generated encoder in box_sdk_gen/networking/multipart_stream.py, which this PR leaves unchanged; it gets the needed fixes through box/box-codegen#995's generated sync PR:

  • text streams such as io.StringIO are encoded as UTF-8 and sent chunked. 4.17.0's encoder fails on them, which worked with requests-toolbelt
  • a length the stream reports through __len__ or len is used before seeking to its end, like requests and requests-toolbelt

Until that sync lands on combined-sdk, test/boxsdk/functional/test_file_upload_update_download.py fails here (36 cases): it uploads through a mocked file that reports a length but can't seek, which 4.17.0's encoder measures as empty. With #995's latest encoder temporarily synced into this branch (a891485, reverted in d8fc6b9), all CI checks passed.

Behaviour changes (for the changelog)

  • requests-toolbelt is no longer installed with the SDK. Code that imports it without declaring it as its own dependency needs to add it.
  • Custom network layers passed as Session(network_layer=...) receive this MultipartStream as data for streamed uploads. It keeps read(), len, content_type and boundary, but no longer has the MultipartEncoder API (fields, to_string(), encoding, boundary_value), and is not a MultipartEncoder instance.
  • Seekable file streams are sent with the same bytes and an exact Content-Length, as before. Non-seekable streams still aren't supported by the legacy session, which seeks every file stream before each attempt; that's unchanged.

Testing

  • run with requests-toolbelt uninstalled (confirmed not importable)
  • legacy unit suite (test/boxsdk/unit): 6403 passed, 2 xfailed (baseline 6400 passed, 3 xfailed)
    • test_multipart_stream.py: replaced the toolbelt-only to_string() with read(), dropped the "Encoder does not support empty fields" xfail since empty input now works, added tests for the exact rendered body (including the content-type tuple) and lazy reading
    • test_session.py: the streaming upload test asserts the rendered body and Content-Type header; test_box_session_seeks_file_after_retry patches out MultipartStream, so it counts only the session's rewinds before each attempt, independent of how the generated encoder sizes streams
  • with box/box-codegen#995's encoder synced:
    • legacy functional suite (test/boxsdk/functional): 195 passed, 11 xfailed (same as combined-sdk)
    • full CI unit command (pytest --disable-pytest-warnings): 6627 passed, 13 xfailed
    • box_sdk_gen network tests: 81 passed
  • uploaded through a legacy Session to a local HTTP server: exact Content-Length, attributes before file, non-ASCII JSON attributes intact, 1 MB file byte-identical
  • test_jwt_auth.py: formatted with black 26.10, which CI now installs and which flagged this untouched file
  • pylint 10.00 on the new module and no new messages in the tests; black --preview --skip-string-normalization --check clean

🤖 Generated with Claude Code

…m encoder

Back the legacy boxsdk MultipartStream with the streaming encoder from
box_sdk_gen.networking.multipart_stream, keeping its (data, files)
constructor and data-before-files ordering. Seekable file streams are now
sent with an exact Content-Length. This removes the requests-toolbelt
dependency from the combined SDK.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@congminh1254
congminh1254 requested a review from a team October 5, 2026 14:32
@congminh1254 congminh1254 changed the title feat(boxsdk): Replace requests-toolbelt with streaming MultipartStream encoder feat(boxsdk): Replace requests-toolbelt with streaming MultipartStream encoder Oct 5, 2026
The format-check job installs the latest black, which now wraps this
yield statement.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@coveralls

coveralls commented Oct 5, 2026 •

Copy link
Copy Markdown

Coverage Report for CI Build 37449479995

Warning

No base build found for commit 1e34853 on combined-sdk.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 97.185%

Details

  • Patch coverage: 15 of 15 lines across 1 file are fully covered (100%).

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →


Coverage Stats

Coverage Status
Relevant Lines: 3765
Covered Lines: 3659
Line Coverage: 97.18%
Coverage Strength: 0.97 hits per line

💛 - Coveralls

congminh1254 and others added 2 commits October 5, 2026 16:48
… in MultipartStream

Sync the generated encoder and its tests with box/box-codegen#995:
- text streams such as io.StringIO are encoded as UTF-8 and sent chunked
- a length the stream reports through __len__ or len is used before
  seeking to its end, like requests and requests-toolbelt

The legacy functional tests upload through a mocked file that reports a
length but can't seek, which was measured as empty and sent no content.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…d length in MultipartStream"

This reverts commit c3cfa44. The generated files come to combined-sdk
through the box/box-codegen#995 sync PR instead.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
lukaszsocha2
lukaszsocha2 previously approved these changes Oct 5, 2026

@lukaszsocha2 lukaszsocha2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm

Patch out MultipartStream, which sizes file streams itself, so the test
checks that the session rewinds file streams before each attempt without
depending on how the generated encoder measures them.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
congminh1254 and others added 2 commits October 5, 2026 19:49
…n#995

Sync the generated encoder and its tests with box/box-codegen#995
(029716b5) to run CI against them. To be reverted once CI passes; the
generated files come to combined-sdk through the #995 sync PR.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…x-codegen#995"

This reverts commit a891485 now that CI passed with it. The generated
files come to combined-sdk through the box/box-codegen#995 sync PR.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@congminh1254 congminh1254 reopened this Oct 6, 2026
@congminh1254
congminh1254 merged commit 685e9fc into combined-sdk Oct 6, 2026
18 checks passed
@congminh1254
congminh1254 deleted the remove-requests-toolbelt-legacy branch October 6, 2026 14:59
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.

3 participants