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

refactor: remove unnecessary compression for GET and DELETE requests#308

Merged
quettabit merged 1 commit intomainfrom
qb/rm-comp-get-del
Feb 14, 2026
Merged

refactor: remove unnecessary compression for GET and DELETE requests#308
quettabit merged 1 commit intomainfrom
qb/rm-comp-get-del

Conversation

@quettabit
Copy link
Member

@quettabit quettabit commented Feb 14, 2026

addresses #301

@quettabit quettabit requested a review from a team as a code owner February 14, 2026 06:30
@greptile-apps
Copy link

greptile-apps bot commented Feb 14, 2026

Greptile Overview

Greptile Summary

Removed compression configuration from GET and DELETE request builders in BaseClient, as these HTTP methods don't send request bodies and compression only applies to request bodies.

  • Removed .compression(self.compression) from get() method (line 740)
  • Removed .compression(self.compression) from delete() method (line 760)
  • POST and PATCH methods retain compression since they send request bodies

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The change correctly removes compression configuration from GET and DELETE methods which don't send request bodies. Compression only applies to request bodies (verified in compress_body function), so this is a logical cleanup that eliminates unnecessary overhead without affecting functionality.
  • No files require special attention

Important Files Changed

Filename Overview
src/api.rs Removed unnecessary compression configuration from GET and DELETE request builders

Flowchart

flowchart TD
    A[BaseClient Request Methods] --> B{HTTP Method Type}
    B -->|GET| C[get method]
    B -->|DELETE| D[delete method]
    B -->|POST| E[post method]
    B -->|PATCH| F[patch method]
    
    C --> G[Set timeout & headers]
    D --> H[Set timeout & headers]
    E --> I[Set timeout, headers & compression]
    F --> J[Set timeout, headers & compression]
    
    G --> K[No body sent]
    H --> L[No body sent]
    I --> M[Body sent with compression]
    J --> N[Body sent with compression]
    
    style C fill:#90EE90
    style D fill:#90EE90
    style E fill:#87CEEB
    style F fill:#87CEEB
    style K fill:#90EE90
    style L fill:#90EE90
    style M fill:#87CEEB
    style N fill:#87CEEB
Loading

Last reviewed commit: 3ebb4cd

@quettabit quettabit merged commit c2215e0 into main Feb 14, 2026
6 checks passed
@quettabit quettabit deleted the qb/rm-comp-get-del branch February 14, 2026 06:36
@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