V10.0.4/ci automation#24
Conversation
📝 WalkthroughWalkthroughThe PR enhances the bump-nuget.py script to automatically update Codebelt packages via NuGet API lookups, adding helper functions to detect Codebelt packages and fetch latest stable versions. It also updates Microsoft.NET.Test.Sdk to version 18.3.0 in Directory.Packages.props. Changes
Sequence DiagramsequenceDiagram
participant Script as Bump Script
participant FS as File System
participant NuGet as NuGet API
participant Props as Directory.Packages.props
Script->>FS: Read Directory.Packages.props
FS-->>Script: Package list with versions
loop For each package
Script->>Script: Check if Codebelt package
alt Is Codebelt Package
Script->>NuGet: Fetch latest stable version
NuGet-->>Script: Version info (cached)
Script->>Script: Compare versions
alt Version newer
Script->>Props: Update package version
Props-->>Script: Confirmation
end
else Triggered Package
Script->>Props: Update to TRIGGER_VERSION
Props-->>Script: Confirmation
else Third-party
Script->>Script: Log as skipped
end
end
Script-->>Script: Return exit code 0
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/scripts/bump-nuget.py (1)
100-107:⚠️ Potential issue | 🟠 MajorValidate
TRIGGER_SOURCEagainst known keys before processing.On Line 100, non-empty values are validated, but an unsupported
TRIGGER_SOURCEis still accepted. That can silently skip the intended triggered-package bump and produce unintended update behavior.Proposed fix
def main(): if not TRIGGER_SOURCE or not TRIGGER_VERSION: print( "Error: TRIGGER_SOURCE and TRIGGER_VERSION environment variables required" ) print(f" TRIGGER_SOURCE={TRIGGER_SOURCE}") print(f" TRIGGER_VERSION={TRIGGER_VERSION}") sys.exit(1) + if TRIGGER_SOURCE not in SOURCE_PACKAGE_MAP: + print(f"Error: Unsupported TRIGGER_SOURCE '{TRIGGER_SOURCE}'") + print(f"Valid sources: {', '.join(sorted(SOURCE_PACKAGE_MAP.keys()))}") + sys.exit(1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/scripts/bump-nuget.py around lines 100 - 107, Add validation after the existing TRIGGER_SOURCE/TRIGGER_VERSION presence check to ensure TRIGGER_SOURCE is one of the supported keys; update the block around the existing if not TRIGGER_SOURCE or not TRIGGER_VERSION: check TRIGGER_SOURCE against the known allowed list (e.g., a constant list/tuple of valid trigger keys used elsewhere in this script) and if it is not present, print a clear error showing the invalid TRIGGER_SOURCE and exit with sys.exit(1). Reference TRIGGER_SOURCE and TRIGGER_VERSION in the same conditional area and ensure the error message explicitly states the allowed values so unsupported keys are rejected before any bump processing occurs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/scripts/bump-nuget.py:
- Around line 91-93: The except block that catches Exception and sets result =
None for package_name swallows lookup failures; update it to record the failure
(e.g., set a failure flag or append package_name to a failures list) or re-raise
after logging so the error isn't treated as success, and ensure the top-level
exit logic (currently always exiting successfully) checks that failure flag/list
and calls sys.exit(1) when any NuGet lookups failed; update the code paths
around result, package_name, and the fetch function (e.g., fetch_latest_version)
so a failed lookup causes a non-zero exit instead of silently passing.
---
Outside diff comments:
In @.github/scripts/bump-nuget.py:
- Around line 100-107: Add validation after the existing
TRIGGER_SOURCE/TRIGGER_VERSION presence check to ensure TRIGGER_SOURCE is one of
the supported keys; update the block around the existing if not TRIGGER_SOURCE
or not TRIGGER_VERSION: check TRIGGER_SOURCE against the known allowed list
(e.g., a constant list/tuple of valid trigger keys used elsewhere in this
script) and if it is not present, print a clear error showing the invalid
TRIGGER_SOURCE and exit with sys.exit(1). Reference TRIGGER_SOURCE and
TRIGGER_VERSION in the same conditional area and ensure the error message
explicitly states the allowed values so unsupported keys are rejected before any
bump processing occurs.
| except Exception as exc: | ||
| print(f" Warning: Could not fetch latest version for {package_name}: {exc}") | ||
| result = None |
There was a problem hiding this comment.
NuGet lookup failures are currently treated as successful runs.
On Lines 91–93, lookup errors are downgraded to warnings; on Line 176, the script always exits with success. This can hide failed package resolution and let CI pass with stale versions.
Proposed fix
_nuget_version_cache: Dict[str, Optional[str]] = {}
+_nuget_lookup_failures: List[str] = []
def get_latest_nuget_version(package_name: str) -> Optional[str]:
@@
except Exception as exc:
print(f" Warning: Could not fetch latest version for {package_name}: {exc}")
+ _nuget_lookup_failures.append(package_name)
result = None
@@
- return 0 if changes else 0 # Return 0 even if no changes (not an error)
+ if _nuget_lookup_failures:
+ print()
+ print(
+ f"Error: Failed to resolve NuGet versions for {len(set(_nuget_lookup_failures))} package(s)."
+ )
+ return 1
+ return 0Also applies to: 176-176
🧰 Tools
🪛 Ruff (0.15.2)
[warning] 91-91: Do not catch blind exception: Exception
(BLE001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/scripts/bump-nuget.py around lines 91 - 93, The except block that
catches Exception and sets result = None for package_name swallows lookup
failures; update it to record the failure (e.g., set a failure flag or append
package_name to a failures list) or re-raise after logging so the error isn't
treated as success, and ensure the top-level exit logic (currently always
exiting successfully) checks that failure flag/list and calls sys.exit(1) when
any NuGet lookups failed; update the code paths around result, package_name, and
the fetch function (e.g., fetch_latest_version) so a failed lookup causes a
non-zero exit instead of silently passing.
There was a problem hiding this comment.
Pull request overview
This PR updates the CI automation used for “service update” workflows by improving the NuGet bumping script to update all Codebelt-related packages (triggered source set to the provided version; other Codebelt packages updated to latest stable from NuGet) and by bumping a test dependency version in central package management.
Changes:
- Enhance
.github/scripts/bump-nuget.pyto update non-triggered Codebelt packages to the latest stable NuGet versions, with caching and clearer output. - Expand
SOURCE_PACKAGE_MAPto include additional Codebelt packages (e.g., Carter, ASP.NET Core Newtonsoft.Json/Yaml, SharedKernel). - Update
Microsoft.NET.Test.Sdkfrom18.0.1to18.3.0inDirectory.Packages.props.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
.github/scripts/bump-nuget.py |
Updates version bumping logic to handle all Codebelt packages (triggered vs. latest-from-NuGet) and adds new supported package prefixes. |
Directory.Packages.props |
Minor central dependency version bump for the test SDK. |
Comments suppressed due to low confidence (1)
.github/scripts/bump-nuget.py:103
- Consider validating that TRIGGER_SOURCE is one of the known keys in SOURCE_PACKAGE_MAP. As-is, an unexpected/typoed TRIGGER_SOURCE will silently disable the “triggered packages set to TRIGGER_VERSION” behavior and may instead bump matching Codebelt packages to whatever NuGet reports as latest.
def main():
if not TRIGGER_SOURCE or not TRIGGER_VERSION:
print(
"Error: TRIGGER_SOURCE and TRIGGER_VERSION environment variables required"
)
| except Exception as exc: | ||
| print(f" Warning: Could not fetch latest version for {package_name}: {exc}") | ||
| result = None | ||
|
|
||
| _nuget_version_cache[package_name] = result |
There was a problem hiding this comment.
On NuGet lookup failure, the exception path sets result=None and the caller silently keeps the existing package version. This can lead to confusing runs (e.g., no updates applied even though an update exists). Consider tracking lookup failures and surfacing them in the summary (or optionally failing the run for unresolved Codebelt packages).
|
This pull request enhances the
.github/scripts/bump-nuget.pyscript to more robustly manage package version updates for Codebelt-related projects. The script now updates all Codebelt packages: those from the triggering source are set to the specified version, while all other Codebelt packages are updated to their latest stable version from NuGet. Additionally, new Codebelt packages are supported, and the script output is clarified. There is also a minor update to theMicrosoft.NET.Test.Sdkpackage version inDirectory.Packages.props.Script enhancements for package version management:
SOURCE_PACKAGE_MAP, includingCodebelt.Extensions.Carter,Codebelt.Extensions.AspNetCore.Newtonsoft.Json,Codebelt.Extensions.AspNetCore.Text.Yaml, andCodebelt.SharedKernel.Minor dependency update:
Microsoft.NET.Test.Sdkversion from18.0.1to18.3.0inDirectory.Packages.props.Summary by CodeRabbit
Release Notes