Skip to content

fix(store): build the S3 condition test store with the read client and retry budget - #54

Merged
yordis merged 1 commit into
mainfrom
yordis/fix-store-s3-test-fields
Oct 29, 2026
Merged

yordis merged 1 commit into
mainfrom
yordis/fix-store-s3-test-fields

Conversation

@yordis

@yordis yordis commented Oct 29, 2026

Copy link
Copy Markdown
Member

…d retry budget

Main stopped compiling its tests once the conditional-request and retry-budget changes landed together.

Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@coderabbitai

coderabbitai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fe04ae44-d2d8-43a6-b7e8-20d3bbe098d0
Commits

Reviewing files that changed from the base of the PR and between 3b5a7e1 and fd7cfcf.

Files selected for processing: 1
  • crates/walgit-store/src/s3.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The S3 test store now initializes both client and read_client from the same configuration and sets max_retries to zero.

Changes

S3 test store setup

Layer / File(s) Summary
Initialize test store clients
crates/walgit-store/src/s3.rs
The test store initializes client and read_client from the same configuration and sets max_retries to zero.

Priority: Normal

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Bug fix

Possibly related PRs

  • TrogonStack/walgit#49: Adds S3 conditional operations; this change updates the condition test store setup to initialize the read client and retry limit.

Suggested reviewers: tobi

Merge Risk: Minimal · up to fd7cf

The test setup now supplies both clients and disables retries. No actionable merge-blocking risk is identified; mergeability remains subject to normal compilation and test checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly identifies the S3 test-store fix and the related read-client and retry-budget changes.
Description check Passed The description explains the test compilation issue caused by combining the conditional-request and retry-budget changes. It directly relates to the changeset.
Docstring Coverage Passed No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.

Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks the clients in a row,
Both share the same config, ready to go.
Retries rest at zero for this test,
The S3 setup is neatly dressed.
The rabbit hops away, pleased with the flow.

Comment @coderabbitai help to get the list of available commands.

@yordis
yordis merged commit fa1dd1a into main Oct 29, 2026
9 of 10 checks passed
@yordis
yordis deleted the yordis/fix-store-s3-test-fields branch October 29, 2026 07:01
@cursor

cursor Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

PR Summary

Low Risk
Test-only change to a mock store constructor; no production S3 behavior is modified.

Overview
Updates the condition_store() test helper so constructed S3Store instances match production after the split between mutation and read clients plus GET retry budget.

The helper now builds separate client and read_client from the same SDK config (via config.clone()), and sets max_retries: 0, restoring test compilation and keeping conditional DELETE/PUT integration tests aligned with real store wiring.

Reviewed by Cursor Bugbot for commit fd7cfcf. Bugbot is set up for automated code reviews on this repo. Configure here.

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.

1 participant