USHIFT-6859: Update generate_common_versions.py to take "X.Y" as argument for 5.0 handling#6505
USHIFT-6859: Update generate_common_versions.py to take "X.Y" as argument for 5.0 handling#6505pmtk wants to merge 1 commit intoopenshift:mainfrom
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@pmtk: This pull request references USHIFT-6859 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughReplaced separate Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI / Caller
participant Shell as common_versions_verify.sh
participant Source as Makefile / Git
participant Py as generate_common_versions.py
participant Gen as generate_common_versions
CI->>Shell: invoke verification
Shell->>Source: read Makefile fields or branch name
Source-->>Shell: return version fields / branch
Shell->>Py: call with positional `version` ("X.Y")
Py->>Py: parse_major_minor(version) -> (major, minor)
Py->>Gen: generate_common_versions(major, minor)
Gen-->>Py: produce/update common_versions.sh
Py-->>Shell: exit status / output
Shell->>CI: diff check result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
bot
added
the
approved
|
@pmtk: This pull request references USHIFT-6859 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
![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)
test/bin/pyutils/generate_common_versions.py (1)
425-426:⚠️ Potential issue | 🔴 Critical
args.majorno longer exists and will crash--create-pr.After switching to positional
version, Line [425] and Line [426] referenceargs.major, which causesAttributeErrorwhen--create-pris used.💡 Proposed fix
- if not base_branch.startswith(f"release-{args.major}"): - logging.error(f"Script is expected to be executed on branch starting with 'release-{args.major}', but it's {base_branch}") + expected_prefix = f"release-{major}.{minor}" + if not base_branch.startswith(expected_prefix): + logging.error(f"Script is expected to be executed on branch starting with '{expected_prefix}', but it's {base_branch}") exit(1)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/bin/pyutils/generate_common_versions.py` around lines 425 - 426, The branch-name check currently references a non-existent args.major (causing AttributeError when running --create-pr); update the code so it derives the major version from args.version (e.g., parse args.version and extract the major component) and use that computed major in the base_branch.startswith call (replace uses of args.major with the new major variable), ensuring the check still reads base_branch.startswith(f"release-{major}") and that args.version is validated before parsing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/bin/pyutils/generate_common_versions.py`:
- Around line 397-408: The CLI currently splits args.version with naive indexing
which can raise IndexError/ValueError and breaks callers that pass just a major
(e.g., `${y}`); update the argument parsing around parser and args.version so it
strictly validates the format and provides a clear argparse error: accept either
"X.Y" or a single "X" (back-compat), validate using a regex or split and
explicit checks, convert to ints only after validation, and call
parser.error(...) on malformed inputs instead of letting raw exceptions
propagate; ensure you still call generate_common_versions(major, minor) with
minor defaulting to 0 for the single-number case, and update the caller
test/bin/common_versions_verify.sh to pass "X.Y" (or rely on the new
backward-compat behavior).
---
Outside diff comments:
In `@test/bin/pyutils/generate_common_versions.py`:
- Around line 425-426: The branch-name check currently references a non-existent
args.major (causing AttributeError when running --create-pr); update the code so
it derives the major version from args.version (e.g., parse args.version and
extract the major component) and use that computed major in the
base_branch.startswith call (replace uses of args.major with the new major
variable), ensuring the check still reads
base_branch.startswith(f"release-{major}") and that args.version is validated
before parsing.
🪄 Autofix (Beta)
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: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 508097f9-45d7-4b97-85ba-657b4c18f7a0
📒 Files selected for processing (1)
test/bin/pyutils/generate_common_versions.py
pmtk
force-pushed
the
common-versions-5.0
branch
from
7ecf033 to
862dc4b
Compare
|
@pmtk: This pull request references USHIFT-6859 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
pmtk
force-pushed
the
common-versions-5.0
branch
from
862dc4b to
bb54432
Compare
|
@pmtk: This pull request references USHIFT-6859 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
pmtk
force-pushed
the
common-versions-5.0
branch
from
bb54432 to
d3d78a7
Compare
|
@pmtk: This pull request references USHIFT-6859 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
test/bin/pyutils/generate_common_versions.py (1)
396-403:⚠️ Potential issue | 🟡 MinorHandle invalid
versionviaparser.error(...)instead of uncaughtArgumentTypeError.Right now malformed input can produce a raw traceback instead of argparse’s normal CLI error output/exit flow.
💡 Proposed fix
def main(): def parse_major_minor(value: str): parts = value.split(".") if len(parts) != 2 or not all(p.isdigit() for p in parts): raise argparse.ArgumentTypeError("version must be in 'X.Y' format") return int(parts[0]), int(parts[1]) parser = argparse.ArgumentParser(description="Generate common_versions.sh variables.") parser.add_argument("version", type=str, help="The major.minor version number.") @@ args = parser.parse_args() - major, minor = parse_major_minor(args.version) + try: + major, minor = parse_major_minor(args.version) + except argparse.ArgumentTypeError as e: + parser.error(str(e)) output = generate_common_versions(major, minor)#!/bin/bash # Verify current parsing/error path wiring in generate_common_versions.py rg -n -C2 'def parse_major_minor|ArgumentTypeError|parser\.add_argument\("version"|parse_major_minor\(args\.version\)|parser\.error\(' test/bin/pyutils/generate_common_versions.pyAlso applies to: 411-412
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/bin/pyutils/generate_common_versions.py` around lines 396 - 403, The parser currently defines parse_major_minor (which raises argparse.ArgumentTypeError) but the argument is registered as type=str and later the script manually calls parse_major_minor(args.version), so malformed input can raise an uncaught ArgumentTypeError; update parser.add_argument("version", ...) to use type=parse_major_minor so argparse handles validation and prints its normal error/exit flow, and for any other direct calls to parse_major_minor (e.g., parse_major_minor(args.version) around lines referencing that call) wrap them in try/except catching argparse.ArgumentTypeError and call parser.error(str(err)) to convert the exception into argparse's error reporting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@test/bin/pyutils/generate_common_versions.py`:
- Around line 396-403: The parser currently defines parse_major_minor (which
raises argparse.ArgumentTypeError) but the argument is registered as type=str
and later the script manually calls parse_major_minor(args.version), so
malformed input can raise an uncaught ArgumentTypeError; update
parser.add_argument("version", ...) to use type=parse_major_minor so argparse
handles validation and prints its normal error/exit flow, and for any other
direct calls to parse_major_minor (e.g., parse_major_minor(args.version) around
lines referencing that call) wrap them in try/except catching
argparse.ArgumentTypeError and call parser.error(str(err)) to convert the
exception into argparse's error reporting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 486b1673-6dee-4ee5-a35c-a3edbcb1e26e
📒 Files selected for processing (2)
test/bin/common_versions_verify.shtest/bin/pyutils/generate_common_versions.py
🚧 Files skipped from review as they are similar to previous changes (1)
- test/bin/common_versions_verify.sh
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pacevedom, pmtk The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/override ci/prow/e2e-aws-tests-bootc-el9 |
|
@pmtk: Overrode contexts on behalf of pmtk: ci/prow/e2e-aws-tests-bootc-el9 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
/verified by @pmtk |
openshift-ci-robot
added
the
verified
|
@pmtk: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@pmtk: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
3 participants
Summary by CodeRabbit
Refactor
Chore