Repository navigation
feat(py-client): Add automatic resumable uploads - #667
Conversation
This comment has been minimized.
This comment has been minimized.
79e839b to
46d4611
Compare
This comment has been minimized.
This comment has been minimized.
Use a 256 MiB source-size threshold and optional size hints. Spool eligible uploads only when needed, preserve caller-owned streams, and fall back to direct uploads when temporary-file creation fails.
Remove size hints and concrete stream type checks. Measure seekable inputs once and spool only for compression. Send non-seekable inputs directly without body replay retries.
Remove the counting reader and use stream positions for size metrics when available. Restore the existing encoding and compression variable names to reduce PR churn.
Handle bytes in get_size and document the two direct-upload fallback paths.
Restore the pre-existing retry policy and remove tests for the extra replay restrictions. Let ValueError propagate when determining input size.
| def prepare_body( | ||
| contents: bytes | IO[bytes], | ||
| source_size: int, | ||
| compress: bool, |
There was a problem hiding this comment.
For a follow-up: This should receive compress_with and then match on the compression algorithm internally. Same for other places where we pass a compress bool.
Replace configurable resumable retries with two retries without progress. Reset the retry count and backoff when the server reports a higher offset, and retry writes that return an unchanged offset.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c50f3fc. Configure here.
| urllib3.exceptions.ReadTimeoutError, | ||
| urllib3.exceptions.ProtocolError, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Resumable retries skip connection failures
Medium Severity
is_transient does not treat exhausted connect retries or HTTP 500 as retryable, so the upload loop cancels the session instead of probing the offset and continuing. Pool connect retries use zero backoff, so a short network blip after progress can discard a large persisted prefix. 502/503/504 and read timeouts are retried, and a probe-before-resend makes retrying these failures safe.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c50f3fc. Configure here.
The client changelog will now have exactly two sections, **Python Client** and **Rust Client**, chosen by the PR title scope. PRs that touch the clients also get a changelog preview comment. Requires craft 3.0.0 or newer, which includes getsentry/craft#919. Older versions add a stray `#### Other` subheading under a client section with 2+ entries. ## What changes for you **PR titles decide the changelog.** For a client change, scope the title: - `feat(py-client): Add automatic resumable uploads` is listed under **Python Client** - `fix(rs-client): Drain discarded response bodies` is listed under **Rust Client** Craft drops the `feat(...)` prefix, so the entry reads "Add automatic resumable uploads by @you in #123". Example output, generated from our real history since `0.2.5` with this config and the craft fix: ```markdown ### Python Client - Add automatic resumable uploads by @lcian in #667 - Honor zero retries by @lcian in #679 - Add private resumable upload API by @lcian in #665 ### Rust Client - Mark Error as non-exhaustive by @lcian in #706 ``` **Changelog preview.** When a PR touches `clients/**`, a bot comments with how the PR will look in the changelog and what bump it implies. The comment updates when you edit the title, description or labels. It's informational only: it never fails or blocks the PR. PRs that don't touch `clients/` get no comment. **Custom wording.** If the title isn't a good user-facing description, add this to the PR description: ```markdown ### Changelog Entry Add `Client::foo` for doing X. ``` That text replaces the title in the changelog. Each top-level bullet becomes a separate entry, all under the section picked by the title's scope. **Leaving a PR out.** Add the `skip-changelog` label, or put `#skip-changelog` anywhere in the PR description. ## Things to be careful of - **Use exactly `py-client` or `rs-client`.** Older variants (`python-client`, `rust-client`, `rust`, `clients`, no scope) aren't matched. A PR that touches `clients/` without a matching scope still goes into the changelog, under a trailing `### Other` section. The preview shows this, so check it. - **Split PRs that change both clients.** Craft puts each PR in one section only, so a combined PR can't appear under both. - **Changes only in `objectstore-types`** don't get a preview and aren't in the changelog. Craft runs from `clients/` and only collects commits that touch `clients/`. If a types-only change matters to users, also touch a client in the PR. ## Changes - `clients/.craft.yml`: `changelogPolicy: auto` becomes `changelog.policy: auto`, which removes the deprecation warning every release logged. Also sets `scopeGrouping: false`. - `clients/.github/release.yml` (new): the Python Client / Rust Client categories. Each title is listed several times so that breaking, `feat` and other changes still produce major, minor and patch bumps. - `.github/workflows/changelog-preview.yml` (new): calls craft's reusable preview workflow, pinned to the `v3` commit, with `working-directory: clients` and a path filter on `clients/**`. It doesn't pass repo secrets (`secrets: inherit`); the called workflow gets `GITHUB_TOKEN` automatically and needs nothing else. - `AGENTS.md`: short PR guidance covering the scopes, splitting PRs, and `### Changelog Entry`. ## Alternative considered: type sections with client subheadings Craft can also keep the usual type sections and put a client subheading inside each: ```markdown ### New Features ✨ #### Python Client - ... ### Bug Fixes 🐛 #### Rust Client - ... ``` That uses the default categories with `scopeGrouping: true`. I didn't use it, for two reasons: - **The scope has to be the full name.** Craft builds the subheading from the title scope by replacing hyphens with spaces and title-casing it, and the label can't be configured. So `py-client` would show up as "Py Client"; to get "Python Client", PR titles would need `python-client` / `rust-client`. - **A single entry gets no subheading.** Craft only adds a scope subheading when that scope has 2+ entries in a section. A lone entry is listed inline as `(python-client) Foo`, and sometimes under a `#### Other`. So the layout would change from release to release. The nesting the other way round (Python Client → Features / Fixes) isn't possible without a craft change: craft always nests type category first, then scope. <!-- junior-request-attribution:start --> via @lcian. <!-- junior-request-attribution:end --> <!-- junior-session-footer:start --> <!-- junior-conversation-id:slack%3AC0C641CPPS8%3A1792657214.524119 --> -- [View Junior Session](https://junior-prod.sentry.dev/conversations/slack%3AC0C641CPPS8%3A1792657214.524119) [[Sentry]](https://sentry.sentry.io/explore/conversations/slack%3AC0C641CPPS8%3A1792657214.524119/?project=4510944073809921) <!-- junior-session-footer:end --> --------- Co-authored-by: sentry-junior[bot] <264270552+sentry-junior[bot]@users.noreply.github.com> Co-authored-by: Lorenzo Cian <[email protected]>


Session.putnow automatically uses resumable uploads under the hood for eligible uploads. An upload is eligible if:resumable_threshold_bytes(default 256 MiB), measured before any client-side compressionbytesor a seekable streamNon-seekable streams fall-back to a regular, non-resumable, PUT.
When on-the-fly compression is needed, we spool the compressed payload to a temporary file before uploading;
Client(spool_dir=...)customizes its directory. Otherwise, we use the buffer or stream directly.Usecasegains the following kwarg, shown with its default:Session.putalso gainsresumable_threshold_bytesfor per-request overrides, defaulting to the value set on theUsecase.Resumable uploads retry transient failures or writes that make no progress up to two times, with exponential backoff with base 2 seconds plus 1 second of jitter.
The retry count and backoff reset whenever a request makes progress.
This is combined with connection retries configured on the urllib3 pool, which are still effective in parallel.
Closes FS-389