USHIFT-6786: Preserve GITOPS_VERSION when access.redhat.com API fetch fails#6476
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@agullon: This pull request references USHIFT-6786 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 "4.22.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. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Walkthrough
Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
bot
added
the
approved
|
@agullon: This pull request references USHIFT-6786 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 "4.22.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.
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)
217-230:⚠️ Potential issue | 🟠 MajorHandle HTTP errors in retry loop to enable fallback.
When
requests.get()succeeds butraise_for_status()throws (4xx/5xx responses), the response object is already assigned toresp. After 3 failed attempts,respis non-None, so the check on line 228 never triggers and the fallback on lines 296-300 never runs. TheGITOPS_VERSIONpreservation fails.Fix: Return from exception handler on final attempt
- resp = None for attempt in range(1, 4): try: resp = requests.get(url, params=params, timeout=10) resp.raise_for_status() break - except Exception as e: - logging.warning(f"Attempt {attempt} failed with error: {e}. Retrying...") - time.sleep(2) - continue - - if resp is None: - logging.error(f"Failed to fetch data from {url} after 3 attempts") - return None + except requests.RequestException as e: + if attempt == 3: + logging.error(f"Failed to fetch data from {url} after 3 attempts: {e}") + return None + logging.warning(f"Attempt {attempt} failed with error: {e}. Retrying...") + time.sleep(2)🤖 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 217 - 230, The retry loop assigns resp before calling resp.raise_for_status(), so when raise_for_status() raises on the final attempt resp remains non-None and the later "if resp is None" check is skipped; update the except block inside the for-loop that wraps requests.get/resp.raise_for_status() to detect the final attempt (when attempt == 3) and immediately log the error and return None (or otherwise exit) so the fallback logic that preserves GITOPS_VERSION can run; refer to the existing variables and calls resp, requests.get(..., timeout=10), resp.raise_for_status(), and the attempt loop to locate where to add the early return.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@test/bin/pyutils/generate_common_versions.py`:
- Around line 217-230: The retry loop assigns resp before calling
resp.raise_for_status(), so when raise_for_status() raises on the final attempt
resp remains non-None and the later "if resp is None" check is skipped; update
the except block inside the for-loop that wraps
requests.get/resp.raise_for_status() to detect the final attempt (when attempt
== 3) and immediately log the error and return None (or otherwise exit) so the
fallback logic that preserves GITOPS_VERSION can run; refer to the existing
variables and calls resp, requests.get(..., timeout=10),
resp.raise_for_status(), and the attempt loop to locate where to add the early
return.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 4818d6bb-b573-4003-b58a-dcb197872e6a
📒 Files selected for processing (1)
test/bin/pyutils/generate_common_versions.py
|
/verified by @agullon |
openshift-ci-robot
added
the
verified
|
@agullon: 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. |
openshift-ci-robot
removed
the
verified
|
/verified by @agullon |
openshift-ci-robot
added
the
verified
|
@agullon: 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. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 217-230: The code currently calls resp.json() and indexes data
outside the retry try/except so malformed JSON or unexpected payloads raise and
bypass retries; move the resp.json() call and basic payload validation into the
existing try block (the loop around requests.get) and catch json.JSONDecodeError
and TypeError/KeyError (or validate that data is a dict with "data" -> list ->
[0] -> "versions") to trigger a retry on parse/shape failures; ensure the
final-attempt branch that logs the failure (using the same message that mentions
url and attempt count) returns None or falls back to the
preserve-existing-version path, and update references to resp, data,
current_microshift_minor_version, and gitops_version_from_api_docs accordingly
so downstream loops only run when data is valid.
🪄 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), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: f27cf37a-954e-41c9-89de-496ff07c9ae3
📒 Files selected for processing (1)
test/bin/pyutils/generate_common_versions.py
When access.redhat.com is unreachable, get_gitops_version() was returning an empty string, causing common_versions.sh to clear GITOPS_VERSION and triggering a spurious PR (e.g. openshift#6475). Return None on API failure to distinguish it from "no compatible version found", and fall back to the existing value in common_versions.sh so the file stays unchanged and no PR is created. pre-commit.check-secrets: ENABLED
openshift-ci-robot
removed
the
verified
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: agullon, 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 |
|
/verified by @agullon |
openshift-ci-robot
added
the
verified
|
@agullon: 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. |
|
/retest |
|
@agullon: 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
access.redhat.comis unreachable,get_gitops_version()returned"", clearingGITOPS_VERSIONincommon_versions.shand creating spurious PRs (e.g. [release-4.22] NO-ISSUE: Update common_versions.sh #6475)Noneon API failure to distinguish from "no compatible version found" ("")GITOPS_VERSIONvalue so the file stays unchanged and no PR is createdJira
USHIFT-6786