Skip to content
This repository was archived by the owner on Feb 16, 2026. It is now read-only.

Comments

feat!: add lower bounds for max_batch_bytes and max_batch_records#309

Merged
quettabit merged 1 commit intomainfrom
qb/batching-min-validation
Feb 14, 2026
Merged

feat!: add lower bounds for max_batch_bytes and max_batch_records#309
quettabit merged 1 commit intomainfrom
qb/batching-min-validation

Conversation

@quettabit
Copy link
Member

@quettabit quettabit commented Feb 14, 2026

addresses #251

@quettabit quettabit force-pushed the qb/batching-min-validation branch from e4ff6d1 to 7d465b3 Compare February 14, 2026 21:12
@quettabit quettabit changed the title [WIP] feat!: validate lower bounds for max_batch_bytes and max_batch_records Feb 14, 2026
@quettabit quettabit marked this pull request as ready for review February 14, 2026 21:19
@quettabit quettabit requested a review from a team as a code owner February 14, 2026 21:19
@greptile-apps
Copy link

greptile-apps bot commented Feb 14, 2026

Greptile Overview

Greptile Summary

This PR adds lower bound validation to with_max_batch_bytes (minimum 8 bytes) and with_max_batch_records (minimum 1 record) methods in BatchingConfig. The changes prevent invalid configuration that could lead to runtime issues or undefined behavior.

Key changes:

  • Introduced RECORD_BATCH_MIN constant using CountOrBytes type
  • Added lower bound checks with clear error messages
  • Improved error message consistency ("must not exceed" vs "exceeds")
  • Updated documentation to reflect both lower and upper bounds
  • Fixed existing test to comply with new 8-byte minimum requirement

Confidence Score: 5/5

  • This PR is safe to merge with no issues found
  • The changes are well-implemented with proper error handling, updated documentation, and adjusted tests. The validation logic is straightforward and prevents invalid configurations. The breaking change is justified and appropriately marked with the feat! prefix.
  • No files require special attention

Important Files Changed

Filename Overview
src/batching.rs Added lower bound validation for batch configuration parameters with proper error messages and updated tests accordingly

Flowchart

flowchart TD
    A[User calls with_max_batch_bytes or with_max_batch_records] --> B{Check lower bound}
    B -->|Below minimum| C[Return ValidationError with lower bound message]
    B -->|Above or equal to minimum| D{Check upper bound}
    D -->|Exceeds maximum| E[Return ValidationError with upper bound message]
    D -->|Within bounds| F[Update config and return Ok]
    
    style C fill:#ff6b6b
    style E fill:#ff6b6b
    style F fill:#51cf66
Loading

Last reviewed commit: 7d465b3

@quettabit quettabit changed the title feat!: validate lower bounds for max_batch_bytes and max_batch_records feat!: add lower bounds for max_batch_bytes and max_batch_records Feb 14, 2026
@quettabit quettabit merged commit d8bfe57 into main Feb 14, 2026
6 checks passed
@quettabit quettabit deleted the qb/batching-min-validation branch February 14, 2026 21:22
@github-actions github-actions bot mentioned this pull request Feb 14, 2026
shikhar pushed a commit that referenced this pull request Feb 15, 2026
## 🤖 New release

* `s2-sdk`: 0.23.8 -> 0.24.0 (⚠ API breaking changes)

### ⚠ `s2-sdk` breaking changes

```text
--- failure inherent_method_missing: pub method removed or renamed ---

Description:
A publicly-visible method or associated fn is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/inherent_method_missing.ron

Failed in:
  CreateBasinInput::with_idempotency_token, previously in file /tmp/.tmpoNf48h/s2-sdk/src/types.rs:913
  CreateStreamInput::with_idempotency_token, previously in file /tmp/.tmpoNf48h/s2-sdk/src/types.rs:2583

--- failure struct_pub_field_missing: pub struct's pub field removed or renamed ---

Description:
A publicly-visible struct has at least one public field that is no longer available under its prior name. It may have been renamed or removed entirely.
        ref: https://doc.rust-lang.org/cargo/reference/semver.html#item-remove
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/struct_pub_field_missing.ron

Failed in:
  field idempotency_token of struct CreateBasinInput, previously in file /tmp/.tmpoNf48h/s2-sdk/src/types.rs:882
  field idempotency_token of struct CreateStreamInput, previously in file /tmp/.tmpoNf48h/s2-sdk/src/types.rs:2561

--- failure struct_pub_field_now_doc_hidden: pub struct field is now #[doc(hidden)] ---

Description:
A pub field of a pub struct is now marked #[doc(hidden)] and is no longer part of the public API.
        ref: https://doc.rust-lang.org/rustdoc/write-documentation/the-doc-attribute.html#hidden
       impl: https://github.com/obi1kenobi/cargo-semver-checks/tree/v0.46.0/src/lints/struct_pub_field_now_doc_hidden.ron

Failed in:
  field CreateBasinInput.idempotency_token in file /tmp/.tmp5yfqnq/s2-sdk-rust/src/types.rs:868
  field CreateStreamInput.idempotency_token in file /tmp/.tmp5yfqnq/s2-sdk-rust/src/types.rs:2539
```

<details><summary><i><b>Changelog</b></i></summary><p>

<blockquote>

## [0.24.0] - 2026-02-15

### Features

- Add accessors for `AppendRecord`
([#305](#305))
- [**breaking**] Add lower bounds for `max_batch_bytes` and
`max_batch_records`
([#309](#309))
- [**breaking**] Reduce default `max_unacked_bytes` to `5MiB`
([#311](#311))

### Refactor

- Replace `reqwest` with `hyper-util` and add client pooling
([#298](#298))
- [**breaking**] Make `idempotency_token` private
([#306](#306))
- [**breaking**] Remove unnecessary `Result` from
`with_max_unacked_batches`
([#307](#307))
- Remove unnecessary compression for GET and DELETE requests
([#308](#308))
- Rename fields, methods, and vars related to `RetryBackoff`
([#310](#310))
- [**breaking**] Make `S2DateTime` conversion from
`time::OffsetDateTime` fallible
([#312](#312))

### Testing

- Metrics
([#297](#297))
- Basin & stream api
([#300](#300))

### Miscellaneous Tasks

- Bump dependencies
([#296](#296))
- Dep updates
([#314](#314))

<!-- generated by git-cliff -->
</blockquote>


</p></details>

---
This PR was generated with
[release-plz](https://github.com/release-plz/release-plz/).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant