Skip to content

Fix update-versions.py script to handle certain cases and add additional tests - #124

Open
Nikhil-Manglore wants to merge 3 commits into
valkey-io:mainlinefrom
Nikhil-Manglore:fix-update-versions
Open

Fix update-versions.py script to handle certain cases and add additional tests#124
Nikhil-Manglore wants to merge 3 commits into
valkey-io:mainlinefrom
Nikhil-Manglore:fix-update-versions

Conversation

@Nikhil-Manglore

Copy link
Copy Markdown
Member

Two commits needed manual updates that we have now patched:

Update 1

Valkey Search released version 1.1.1. In Valkey Bundle version 9.1, Search version 1.2.1 already existed so the Valkey Search version correctly didn't update. However the overall Valkey Bundle version still updated from 9.1.1 to 9.1.2. We now handle cases where we only update the Valkey Bundle version if a module or core version is actually bumped in each version block

Update 2

Valkey 8.1.9 released and its GitHub release event fired the release workflow twice, resulting in two dispatches into Valkey Bundle for the same version. The first run correctly bumped the 8.1 block from 8.1.8 to 8.1.9, but the second run bumped it again to 8.1.10 even though nothing had actually changed. Separately, when the Valkey 9.1.1 update ran, the 9.1 block's Valkey Bundle version failed to bump from 9.1.1 to 9.1.2 because the script was skipping the bump whenever the batching branch (valkey-bundle-update) already existed, regardless of whether that specific block had been touched. Both issues are now fixed. We now compares each block's current bundle version to its value on mainline, so a block that hasn't been touched yet on the branch bumps once and a block that has been touched doesn't double-bump on a repeat dispatch.

Signed-off-by: Nikhil Manglore <nmanglor@amazon.com>
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@Nikhil-Manglore, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 40 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8dbaaf3a-2082-4354-8e2e-1bc2e0501b63

📥 Commits

Reviewing files that changed from the base of the PR and between a9191f3 and 92b4b00.

📒 Files selected for processing (2)
  • scripts/update-versions.py
  • tests/test_update_versions.py
📝 Walkthrough

Walkthrough

The update script now retrieves bundle versions from origin/mainline. Valkey updates use explicit mainline comparison rules. Module updates track changes in the latest block before bumping its bundle. Tests cover duplicate prevention, backports, releases, and multi-block updates.

Changes

Bundle version updates

Layer / File(s) Summary
Mainline lookup and Valkey bump rules
scripts/update-versions.py, tests/test_update_versions.py
The script reads bundle versions from origin/mainline:versions.json. Valkey updates apply separate backport, RC, stable, and mainline-matching rules. Tests replace open-branch mocks with mainline version mocks.
Module change tracking and validation
scripts/update-versions.py, tests/test_update_versions.py
Module updates record whether the latest block changed. The latest bundle is bumped only when required. Tests cover duplicate prevention, unmatched patches, GA downgrades, and multi-block updates.

Sequence Diagram(s)

sequenceDiagram
  participant UpdateScript as update-versions.py
  participant Mainline as origin/mainline
  participant Versions as versions.json
  participant BundleRules as bundle update rules

  UpdateScript->>Mainline: Request bundle version
  Mainline-->>Versions: Read versions.json
  Versions-->>UpdateScript: Return mainline bundle version
  UpdateScript->>BundleRules: Compare current and mainline versions
  BundleRules-->>UpdateScript: Apply Valkey or module bump rules
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the update-versions.py fixes and the addition of tests.
Description check ✅ Passed The description directly explains the version-bump issues, fixes, and test coverage in the changeset.
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.

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@scripts/update-versions.py`:
- Around line 74-80: Update scripts/update-versions.py lines 74-80 in
get_mainline_bundle_version to return a distinct unavailable-data result when
origin/mainline or the requested block cannot be read, and ensure callers stop
mutation for that result while preserving a separate value for an available
block with no prior bump. Update tests/test_update_versions.py lines 84-94 so
the mock returns each block’s current version for equality testing, and add
separate coverage for unavailable mainline data.

In `@tests/test_update_versions.py`:
- Around line 84-94: Update _mainline_matches_current to return
versions_data[block]["version"] for existing blocks, so it models mainline
equality with the current bundle version instead of a missing lookup. Add a
separate test for mainline lookup failure, after updating the production path to
handle that failure safely.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 1417b9d4-7cb2-4655-9960-997d3965ed94

📥 Commits

Reviewing files that changed from the base of the PR and between a9191f3 and 2c56e84.

📒 Files selected for processing (2)
  • scripts/update-versions.py
  • tests/test_update_versions.py

Comment thread scripts/update-versions.py Outdated
Comment thread tests/test_update_versions.py Outdated
Signed-off-by: Nikhil Manglore <nmanglor@amazon.com>
Signed-off-by: Nikhil Manglore <nmanglor@amazon.com>
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