fix(object_store): merge per-request timeout and retry overrides

[shikhar]

Summary

• merge per-request timeout overrides with the existing S3 client timeout config instead of replacing it
• merge per-request retry overrides with the existing S3 client retry config instead of resetting to SDK defaults
• apply timeout/retry config overrides independently so unset categories are left untouched
• add unit tests for merge behavior and no-op behavior

Testing

• cargo +nightly fmt
• cargo clippy --all-features --all-targets -- -D warnings --allow deprecated
• cargo nextest run --lib
• cargo nextest run (fails in this environment because Docker/MinIO is unavailable: connection refused to container runtime)

Closes #41

[greptile-apps]

Greptile Summary

Fixes per-request timeout and retry config overrides to merge on top of the existing S3 client configuration instead of replacing it. Previously, setting any per-request override (e.g., just connect_timeout) would wipe all other timeout/retry settings from the client config. Now, merged_timeout_config() and merged_retry_config() start from the client's base config and only override the fields explicitly set in the request, leaving unset fields untouched. The two config categories (timeout and retry) are also applied independently — setting only retry overrides no longer resets timeout config to defaults, and vice versa.

• Renamed timeout_config() → merged_timeout_config() and retry_config() → merged_retry_config() with Option return types to signal when no merge is needed
• Added has_timeout_overrides() / has_retry_overrides() helpers to early-return None when a category has no overrides
• Updated Downloader::attempt_inner() to conditionally apply each config category to the override builder
• Added unit tests verifying merge preservation of base fields, fallback to SDK defaults, and no-op behavior

Confidence Score: 5/5

• This PR is safe to merge — it's a focused bug fix with correct merge semantics and good test coverage.
• The changes are minimal, well-scoped, and address a clear bug (config replacement vs. merge). The logic is straightforward: start from base config, overlay only the set fields. The helper predicates correctly mirror the field partitioning in is_noop(). Unit tests cover the key scenarios. No new dependencies are introduced, and the public API change is purely internal to the crate.
• No files require special attention.

Important Files Changed

Filename	Overview
src/object_store/config.rs	Refactors timeout_config() and retry_config() into merged_timeout_config() and merged_retry_config() that merge per-request overrides onto an existing base config instead of replacing it. Adds helper predicates has_timeout_overrides() and has_retry_overrides() for early-return when no overrides exist. Includes thorough unit tests covering merge preservation, no-base fallback, and no-op behavior.
src/object_store/downloader.rs	Updates attempt_inner() to use the new merge-based config methods. Reads the existing client config and conditionally applies timeout and retry overrides independently, so unset categories preserve the client's original configuration.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[attempt_inner called with RequestConfig] --> B{is_noop?}
    B -->|Yes| C[Send request with default client config]
    B -->|No| D[Read client base config]
    D --> E[Create config override builder from base]
    E --> F{has_timeout_overrides?}
    F -->|Yes| G[Merge timeout overrides onto base TimeoutConfig]
    F -->|No| H[Leave timeout config unchanged]
    G --> I{has_retry_overrides?}
    H --> I
    I -->|Yes| J[Merge retry overrides onto base RetryConfig]
    I -->|No| K[Leave retry config unchanged]
    J --> L[Send request with merged config_override]
    K --> L

Loading

_{Last reviewed commit: d2d43ab}