refactor(constants): split utilities/constants.py into submodule package#5188
Open
vsibirsk wants to merge 8 commits into
Open
refactor(constants): split utilities/constants.py into submodule package#5188vsibirsk wants to merge 8 commits into
vsibirsk wants to merge 8 commits into
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
🧰 Additional context used📓 Path-based instructions (3)**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
**⚙️ CodeRabbit configuration file
Files:
⚙️ CodeRabbit configuration file
Files:
tests/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (69)📚 Learning: 2026-01-12T11:24:13.825ZApplied to files:
📚 Learning: 2026-01-12T14:25:05.723ZApplied to files:
📚 Learning: 2026-01-20T01:03:13.139ZApplied to files:
📚 Learning: 2026-01-21T21:26:41.805ZApplied to files:
📚 Learning: 2026-01-25T13:18:21.675ZApplied to files:
📚 Learning: 2026-02-18T06:35:39.536ZApplied to files:
📚 Learning: 2026-02-23T16:33:22.070ZApplied to files:
📚 Learning: 2026-03-17T01:32:02.617ZApplied to files:
📚 Learning: 2026-03-17T01:32:02.617ZApplied to files:
📚 Learning: 2026-05-04T13:45:29.122ZApplied to files:
📚 Learning: 2026-05-04T13:45:33.892ZApplied to files:
📚 Learning: 2026-05-05T17:01:15.294ZApplied to files:
📚 Learning: 2026-05-08T12:49:20.694ZApplied to files:
📚 Learning: 2026-05-12T05:10:24.601ZApplied to files:
📚 Learning: 2026-05-13T19:23:09.603ZApplied to files:
📚 Learning: 2026-05-18T06:30:56.781ZApplied to files:
📚 Learning: 2026-05-18T06:31:12.015ZApplied to files:
📚 Learning: 2026-05-18T06:31:15.083ZApplied to files:
📚 Learning: 2026-05-18T06:31:20.848ZApplied to files:
📚 Learning: 2026-05-18T09:09:09.479ZApplied to files:
📚 Learning: 2026-05-19T07:48:17.119ZApplied to files:
📚 Learning: 2026-05-19T07:48:17.119ZApplied to files:
📚 Learning: 2026-05-26T15:52:31.613ZApplied to files:
📚 Learning: 2026-02-18T06:34:38.042ZApplied to files:
📚 Learning: 2026-05-03T15:38:09.624ZApplied to files:
📚 Learning: 2026-05-15T18:42:02.504ZApplied to files:
📚 Learning: 2026-06-15T10:56:21.758ZApplied to files:
📚 Learning: 2025-12-15T12:33:06.686ZApplied to files:
📚 Learning: 2026-01-14T04:08:23.032ZApplied to files:
📚 Learning: 2026-01-18T09:44:17.044ZApplied to files:
📚 Learning: 2026-01-27T17:18:49.973ZApplied to files:
📚 Learning: 2026-02-23T16:31:34.505ZApplied to files:
📚 Learning: 2026-02-25T10:52:09.679ZApplied to files:
📚 Learning: 2026-03-19T10:36:59.023ZApplied to files:
📚 Learning: 2026-03-25T11:24:07.687ZApplied to files:
📚 Learning: 2026-03-31T08:35:22.802ZApplied to files:
📚 Learning: 2026-03-31T10:39:33.409ZApplied to files:
📚 Learning: 2026-04-02T09:03:57.004ZApplied to files:
📚 Learning: 2026-04-14T13:00:57.514ZApplied to files:
📚 Learning: 2026-04-14T16:15:31.065ZApplied to files:
📚 Learning: 2026-04-14T16:15:33.012ZApplied to files:
📚 Learning: 2026-04-21T19:08:39.771ZApplied to files:
📚 Learning: 2026-04-26T11:44:20.150ZApplied to files:
📚 Learning: 2026-04-27T15:40:31.167ZApplied to files:
📚 Learning: 2026-05-03T14:47:13.096ZApplied to files:
📚 Learning: 2026-05-05T17:27:32.109ZApplied to files:
📚 Learning: 2026-05-05T18:28:01.097ZApplied to files:
📚 Learning: 2026-05-07T12:34:42.589ZApplied to files:
📚 Learning: 2026-05-08T12:31:26.895ZApplied to files:
📚 Learning: 2026-05-12T18:18:20.607ZApplied to files:
📚 Learning: 2026-05-25T09:13:27.011ZApplied to files:
📚 Learning: 2026-05-25T09:57:03.042ZApplied to files:
📚 Learning: 2026-05-28T12:55:07.435ZApplied to files:
📚 Learning: 2026-05-29T07:28:31.170ZApplied to files:
📚 Learning: 2026-06-01T13:36:34.844ZApplied to files:
📚 Learning: 2026-06-01T14:00:39.271ZApplied to files:
📚 Learning: 2026-06-02T21:16:58.118ZApplied to files:
📚 Learning: 2026-06-11T11:56:02.760ZApplied to files:
📚 Learning: 2026-06-15T11:26:05.202ZApplied to files:
📚 Learning: 2025-12-22T16:27:40.244ZApplied to files:
📚 Learning: 2026-01-07T09:52:12.342ZApplied to files:
📚 Learning: 2026-01-18T13:18:48.808ZApplied to files:
📚 Learning: 2026-01-18T14:51:50.846ZApplied to files:
📚 Learning: 2026-01-29T05:30:13.982ZApplied to files:
📚 Learning: 2026-02-02T17:41:12.759ZApplied to files:
📚 Learning: 2026-02-03T07:34:34.184ZApplied to files:
📚 Learning: 2026-02-10T15:04:14.799ZApplied to files:
📚 Learning: 2026-02-25T11:00:02.013ZApplied to files:
📚 Learning: 2026-03-29T13:51:25.599ZApplied to files:
🔇 Additional comments (2)
📝 WalkthroughWalkthroughConverts monolithic ChangesConstants Package Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
Contributor
|
Report bugs in Issues Welcome! 🎉This pull request will be automatically processed with the following features: 🔄 Automatic Actions
📋 Available CommandsPR Status Management
Review & Approval
Testing & Validation
Container Operations
Cherry-pick Operations
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
openshift-virtualization-qe-bot-2
requested review from
OhadRevah,
RoniKishner,
geetikakay,
hmeir and
rnetser
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5188 +/- ##
==========================================
- Coverage 98.67% 98.65% -0.02%
==========================================
Files 25 42 +17
Lines 2487 2451 -36
==========================================
- Hits 2454 2418 -36
Misses 33 33
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@utilities/constants/aaq.py`:
- Around line 20-26: Replace the incorrect memory request value in the
QUOTA_FOR_POD mapping: currently REQUESTS_MEMORY_STR is set to
POD_LIMITS_MEMORY; change it to POD_REQUESTS_MEMORY so requests reflect the
intended 2.5Gi value (update the QUOTA_FOR_POD dict where REQUESTS_MEMORY_STR is
defined to use POD_REQUESTS_MEMORY instead of POD_LIMITS_MEMORY).
In `@utilities/constants/components.py`:
- Line 202: Replace the list concatenation for ALL_CNV_PODS with iterable
unpacking: instead of using CNV_PODS_NO_HPP_CSI_HPP_POOL +
[HOSTPATH_PROVISIONER_CSI, HPP_POOL], construct ALL_CNV_PODS by expanding
CNV_PODS_NO_HPP_CSI_HPP_POOL with the two items (HOSTPATH_PROVISIONER_CSI,
HPP_POOL) using [*CNV_PODS_NO_HPP_CSI_HPP_POOL, HOSTPATH_PROVISIONER_CSI,
HPP_POOL] to improve readability and follow the suggested style.
In `@utilities/constants/monitoring.py`:
- Around line 1-4: Add a module-level docstring to this constants module
explaining its purpose and boundaries (what kinds of monitoring level constants
it contains) to match other files in utilities/constants; update the top of the
file where CRITICAL_STR, INFO_STR, WARNING_STR, and NONE_STRING are defined by
inserting a clear one- or two-sentence docstring that states this module holds
monitoring level string constants and any conventions (e.g., allowed values or
usage scope).
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 46829ede-9043-4711-8ea1-d1ed0e21b096
📒 Files selected for processing (19)
tests/global_config.pytests/install_upgrade_operators/product_upgrade/utils.pytests/install_upgrade_operators/relationship_labels/constants.pyutilities/constants.pyutilities/constants/__init__.pyutilities/constants/aaq.pyutilities/constants/components.pyutilities/constants/hco.pyutilities/constants/images.pyutilities/constants/instance_types.pyutilities/constants/misc.pyutilities/constants/monitoring.pyutilities/constants/namespaces.pyutilities/constants/networking.pyutilities/constants/os_matrix.pyutilities/constants/storage.pyutilities/constants/timeouts.pyutilities/constants/virt.pyutilities/unittests/test_constants.py
💤 Files with no reviewable changes (1)
- utilities/constants.py
geetikakay
reviewed
Jun 11, 2026
geetikakay
reviewed
Jun 11, 2026
geetikakay
reviewed
Jun 11, 2026
rnetser
requested changes
Jun 14, 2026
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
COMMIT_SHA=$(gh pr view 5188 --repo RedHatQE/openshift-virtualization-tests --json headRefOid --jq '.headRefOid')
echo "HEAD SHA: $COMMIT_SHA"
# Get the first changed file and first changed line in the diff
gh pr diff 5188 --repo RedHatQE/openshift-virtualization-tests | head -30Length of output: 1563 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - << 'PYEOF'
import json, os, urllib.request, sys
token = os.popen("gh auth token").read().strip()
commit_sha = "f00b38fd8a7f90fbc27f1fdc199323a77d3f61da"
body = (
"## Test Execution Plan\n\n"
"**Run smoke tests: True**\n"
"Dependency path: every smoke test (e.g. `tests/virt/node/general/test_container_disk_vm.py`) "
"-> `tests/conftest.py` (line 79: `from utilities.constants import ...`) "
"-> `utilities/constants/__init__.py` (restructured package). "
"`tests/conftest.py` is in the conftest hierarchy of **all** tests in the repo; "
"any import-time failure in `utilities/constants` would break collection for all smoke tests.\n\n"
"**Run gating tests: True**\n"
"Concrete dependency paths:\n"
"- `tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py` (gating) "
"-> `tests/install_upgrade_operators/relationship_labels/constants.py` imports `VIRTCTL_CLI_DOWNLOADS` "
"from `utilities.constants` (renamed from `VIRTCTL_CLIDOWNLOADS_KUBEVIRT_HYPERCONVERGED`)\n"
"- `tests/install_upgrade_operators/product_upgrade/test_upgrade.py` (gating + cnv_upgrade) "
"-> `tests/install_upgrade_operators/product_upgrade/utils.py` imports `BREW_REGISTRY_SOURCE` "
"(typo fix from `BREW_REGISTERY_SOURCE`)\n"
"- All remaining gating tests are transitively affected via `tests/conftest.py`\n\n"
"---\n\n"
"**Affected tests to run**\n\n"
"| Path | Marker |\n"
"|------|--------|\n"
"| `utilities/unittests/test_constants.py` | unit |\n"
"| `utilities/unittests/test_oadp.py` | unit |\n"
"| `tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py` | gating |\n"
"| `tests/install_upgrade_operators/product_upgrade/test_upgrade.py` | gating, cnv_upgrade |\n"
"| `tests/install_upgrade_operators/deployment/test_hco_deployment_params.py` | gating, smoke |\n"
"| `tests/install_upgrade_operators/hco_enablement_golden_image_updates/test_custom_golden_images_namespace.py` | gating, smoke |\n"
"| `tests/infrastructure/instance_types/supported_os/test_rhel_os.py` | smoke, gating |\n"
"| `tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py` | smoke |\n"
"| `tests/storage/cdi_upload/test_upload_virtctl.py` | smoke |\n"
"| `tests/storage/cdi_clone/test_clone.py` | smoke, gating |\n\n"
"---\n\n"
"**Real tests (cluster required)**\n\n"
"`utilities/constants/__init__.py` calls `_get_cluster_architecture()` at **module load time** "
"to compute `Images`. Every test session triggers this at collection time. "
"Verify before running full suites:\n\n"
"Collection startup check (confirms no `ImportError`/`AttributeError`):\n"
"```\n"
"pytest tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py --collect-only\n"
"pytest tests/install_upgrade_operators/product_upgrade/test_upgrade.py --collect-only\n"
"```\n\n"
"Renamed constant regression (`VIRTCTL_CLI_DOWNLOADS`):\n"
"```\n"
"pytest tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py -m gating\n"
"```\n\n"
"Typo-fix regression (`BREW_REGISTRY_SOURCE`):\n"
"```\n"
"pytest tests/install_upgrade_operators/product_upgrade/test_upgrade.py -m 'gating and cnv_upgrade'\n"
"```\n"
)
payload = {
"commit_id": commit_sha,
"event": "COMMENT",
"body": "",
"comments": [
{
"path": "tests/global_config.py",
"line": 18,
"side": "RIGHT",
"body": body
}
]
}
with open("/tmp/review_payload.json", "w") as f:
json.dump(payload, f)
data = json.dumps(payload).encode("utf-8")
req = urllib.request.Request(
"https://api.github.com/repos/RedHatQE/openshift-virtualization-tests/pulls/5188/reviews",
data=data,
headers={
"Authorization": "Bearer " + token,
"Accept": "application/vnd.github+json",
"Content-Type": "application/json",
"X-GitHub-Api-Version": "2022-11-28",
},
method="POST"
)
try:
with urllib.request.urlopen(req) as resp:
result = json.loads(resp.read())
print("SUCCESS: review id=" + str(result.get("id")) + ", url=" + str(result.get("html_url")))
except urllib.error.HTTPError as e:
body_err = e.read().decode()
print("HTTP ERROR " + str(e.code) + ": " + body_err)
sys.exit(1)
PYEOFLength of output: 7292 🏁 Script executed: #!/bin/bash
set -euo pipefail
COMMIT_SHA="f00b38fd8a7f90fbc27f1fdc199323a77d3f61da"
python3 -c "
import json
body = '''## Test Execution Plan
**Run smoke tests: True**
Dependency path: every smoke test (e.g. \`tests/virt/node/general/test_container_disk_vm.py\`) -> \`tests/conftest.py\` (line 79: \`from utilities.constants import ...\`) -> \`utilities/constants/__init__.py\` (restructured package). \`tests/conftest.py\` is in the conftest hierarchy of **all** tests in the repo; any import-time failure in \`utilities/constants\` would break collection for all smoke tests.
**Run gating tests: True**
Concrete dependency paths:
- \`tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py\` (gating) -> \`tests/install_upgrade_operators/relationship_labels/constants.py\` imports \`VIRTCTL_CLI_DOWNLOADS\` from \`utilities.constants\` (renamed from \`VIRTCTL_CLIDOWNLOADS_KUBEVIRT_HYPERCONVERGED\`)
- \`tests/install_upgrade_operators/product_upgrade/test_upgrade.py\` (gating + cnv_upgrade) -> \`tests/install_upgrade_operators/product_upgrade/utils.py\` imports \`BREW_REGISTRY_SOURCE\` (typo fix from \`BREW_REGISTERY_SOURCE\`)
- All remaining gating tests are transitively affected via \`tests/conftest.py\`
---
**Affected tests to run**
| Path | Marker |
|------|--------|
| \`utilities/unittests/test_constants.py\` | unit |
| \`utilities/unittests/test_oadp.py\` | unit |
| \`tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py\` | gating |
| \`tests/install_upgrade_operators/product_upgrade/test_upgrade.py\` | gating, cnv_upgrade |
| \`tests/install_upgrade_operators/deployment/test_hco_deployment_params.py\` | gating, smoke |
| \`tests/install_upgrade_operators/hco_enablement_golden_image_updates/test_custom_golden_images_namespace.py\` | gating, smoke |
| \`tests/infrastructure/instance_types/supported_os/test_rhel_os.py\` | smoke, gating |
| \`tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py\` | smoke |
| \`tests/storage/cdi_upload/test_upload_virtctl.py\` | smoke |
| \`tests/storage/cdi_clone/test_clone.py\` | smoke, gating |
---
**Real tests (cluster required)**
\`utilities/constants/__init__.py\` calls \`_get_cluster_architecture()\` at **module load time** to compute \`Images\`. Every test session triggers this at collection time. Verify before running full suites:
Collection startup check (confirms no \`ImportError\`/\`AttributeError\`):
\`\`\`
pytest tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py --collect-only
pytest tests/install_upgrade_operators/product_upgrade/test_upgrade.py --collect-only
\`\`\`
Renamed constant regression (\`VIRTCTL_CLI_DOWNLOADS\`):
\`\`\`
pytest tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py -m gating
\`\`\`
Typo-fix regression (\`BREW_REGISTRY_SOURCE\`):
\`\`\`
pytest tests/install_upgrade_operators/product_upgrade/test_upgrade.py -m 'gating and cnv_upgrade'
\`\`\`'''
payload = {
'commit_id': 'f00b38fd8a7f90fbc27f1fdc199323a77d3f61da',
'event': 'COMMENT',
'body': '',
'comments': [
{
'path': 'tests/global_config.py',
'line': 18,
'side': 'RIGHT',
'body': body
}
]
}
with open('/tmp/review_payload.json', 'w') as f:
json.dump(payload, f)
print('payload written')
"
gh api \
--method POST \
-H "Accept: application/vnd.github+json" \
-H "X-GitHub-Api-Version: 2022-11-28" \
/repos/RedHatQE/openshift-virtualization-tests/pulls/5188/reviews \
--input /tmp/review_payload.json \
--jq '"Review posted: id=\(.id) url=\(.html_url)"'Length of output: 3844 |
Contributor
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #5219. Overlapping filesutilities/oadp.py |
Contributor
|
/retest all Auto-triggered: Files in this PR were modified by merged PR #5167. Overlapping filestests/install_upgrade_operators/product_upgrade/utils.py |
Replace the monolithic utilities/constants.py (1095 lines) with a utilities/constants/ package of 13 thematic submodules. A temporary __init__.py compat shim re-exports every public name so all existing `from utilities.constants import X` statements continue to work unchanged while per-team import migrations land in follow-up PRs. Submodules: aaq, components, hco, images, instance_types, misc, monitoring, namespaces, networking, os_matrix, storage, timeouts, virt. Bundled cleanup: - Drop 7 unused constants (TIMEOUT_35MIN, TIMEOUT_40SEC, TIMEOUT_90MIN, TIMEOUT_12HRS, NONE_STR, KUBEVIRT_VIRT_OPERATOR_UP, HOSTPATH_CSI_BASIC) - Fix typo BREW_REGISTERY_SOURCE -> BREW_REGISTRY_SOURCE (3 files) - Dedup VIRTCTL_CLIDOWNLOADS_KUBEVIRT_HYPERCONVERGED -> VIRTCTL_CLI_DOWNLOADS Generated-by: Cursor Agent (Claude Sonnet 4.6) Signed-off-by: vsibirsk <vsibirsk@redhat.com>
Assisted-by: Cursor Agent (Claude Sonnet 4.6) Signed-off-by: vsibirsk <vsibirsk@redhat.com>
Update all callers (utilities/oadp.py, utilities/unittests/test_oadp.py) to use NamespacesNames.ADP_NAMESPACE directly. Remove the module-level name entirely — no compat alias needed. Assisted-by: Cursor Agent (Claude Sonnet 4.6) Signed-off-by: vsibirsk <vsibirsk@redhat.com>
Signed-off-by: vsibirsk <vsibirsk@redhat.com>
- Split misc.py into 4 new themed modules: architecture, cluster, oadp, pytest - Distribute remaining misc constants to virt, networking, monitoring, storage, images - Move node labels (CPU_MODEL_LABEL_PREFIX, TSC_FREQUENCY, NODE_TYPE_WORKER_LABEL) from components.py to cluster.py; group components.py by type with section headers - Update __init__.py shim accordingly Generated-by: Cursor Agent (Claude Sonnet 4.6) Signed-off-by: vsibirsk <vsibirsk@redhat.com>
Split virt.py further: KUBERNETES_ARCH_LABEL → cluster.py, Tekton constants → tekton.py, CPU model exclusion lists → cpu_models.py, WINDOWS_*_PREFERENCE → instance_types.py. Update __init__.py and all module docstrings to reflect the final layout. Generated-by: Claude Sonnet 4.6 Signed-off-by: vsibirsk <vsibirsk@redhat.com>
Signed-off-by: vsibirsk <vsibirsk@redhat.com>
vsibirsk
force-pushed
the
vk-refactor-constants
branch
from
f00b38f to
7f431e7
Compare
Collaborator
Author
|
/verified change: rebase to resolve merge conflict |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
Contributor
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Output rules: Your deliverable: Test Execution Plan
Use these formats:
Real test commands (MANDATORY when changes affect session/runtime code): When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
Example output for a session-startup fix: Guidelines:
Hardware-Related Checks (SR-IOV, GPU, DPDK): When PR modifies fixtures for hardware-specific resources:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL — Verification rules for smoke and gating decisions: |
Collaborator
|
/retest verify-bugs-are-open |
Collaborator
|
/approve |
Collaborator
|
@coderabbitai |
Contributor
|
✅ Action performedFull review finished. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@utilities/constants/namespaces.py`:
- Line 17: The MACHINE_API_NAMESPACE constant in the namespaces module has an
incorrect value of "machine-api-namespace" and is currently unused throughout
the codebase. Either remove the MACHINE_API_NAMESPACE constant entirely if it is
not needed, or update its value to the correct OpenShift namespace
"openshift-machine-api" and add usages in the code where the machine API
namespace is actually required, such as in Kubernetes API calls or resource
references that interact with the machine API.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 12d12c2a-9833-4609-90c0-716a1621121e
📒 Files selected for processing (25)
tests/global_config.pytests/install_upgrade_operators/relationship_labels/constants.pyutilities/constants.pyutilities/constants/__init__.pyutilities/constants/aaq.pyutilities/constants/architecture.pyutilities/constants/cluster.pyutilities/constants/components.pyutilities/constants/cpu_models.pyutilities/constants/hco.pyutilities/constants/images.pyutilities/constants/instance_types.pyutilities/constants/monitoring.pyutilities/constants/namespaces.pyutilities/constants/networking.pyutilities/constants/oadp.pyutilities/constants/os_matrix.pyutilities/constants/pytest.pyutilities/constants/storage.pyutilities/constants/tekton.pyutilities/constants/timeouts.pyutilities/constants/virt.pyutilities/oadp.pyutilities/unittests/test_constants.pyutilities/unittests/test_oadp.py
💤 Files with no reviewable changes (1)
- utilities/constants.py
Add OPENSHIFT_CNV, OPENSHIFT_MARKETPLACE, OPENSHIFT_SRIOV_NETWORK_OPERATOR, fix MACHINE_API_NAMESPACE value, replace five hardcoded strings in global_config.py. Generated-by: Claude Sonnet 4.6 Signed-off-by: vsibirsk <vsibirsk@redhat.com>
Collaborator
Author
|
/verified
|
Contributor
Test execution plan request detailsCRITICAL: You MUST post an inline review comment on the first changed line of the first file. As an expert software testing engineer, analyze all modified files in this PR and create a targeted test execution plan. Analysis Requirements:
Output rules: Your deliverable: Test Execution Plan
Use these formats:
Real test commands (MANDATORY when changes affect session/runtime code): When the affected code runs at session/collection time (conftest fixtures, pytest plugins,
Example output for a session-startup fix: Guidelines:
Hardware-Related Checks (SR-IOV, GPU, DPDK): When PR modifies fixtures for hardware-specific resources:
CRITICAL WORKFLOW COMPLETION RULES: When responding to this test execution plan request, you MUST follow these rules EXACTLY:
ABSOLUTE PROHIBITIONS (violating these creates empty/meaningless reviews):
Remember: The pull request review is visible to users. Additional comments are redundant noise. CRITICAL — Verification rules for smoke and gating decisions: |
| ALL_CNV_PODS, | ||
| ALL_HCO_RELATED_OBJECTS, | ||
| BREW_REGISTERY_SOURCE, | ||
| BREW_REGISTRY_SOURCE, |
Contributor
There was a problem hiding this comment.
Test Execution Plan
Run smoke tests: True
Dependency path: every smoke test (e.g. tests/virt/node/general/test_container_disk_vm.py) -> tests/conftest.py (from utilities.constants import ...) -> utilities/constants/__init__.py (restructured package, calls _get_cluster_architecture() at import time). tests/conftest.py is in the conftest hierarchy of all tests in the repo; any import-time failure in utilities/constants breaks collection for all smoke tests.
Run gating tests: True
Concrete dependency paths:
tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py(gating) ->tests/install_upgrade_operators/relationship_labels/constants.pyimportsVIRTCTL_CLI_DOWNLOADSfromutilities.constants(renamed fromVIRTCTL_CLIDOWNLOADS_KUBEVIRT_HYPERCONVERGED)tests/install_upgrade_operators/product_upgrade/test_upgrade.py(gating, cnv_upgrade) ->tests/install_upgrade_operators/product_upgrade/utils.pyimportsBREW_REGISTRY_SOURCE(typo fix fromBREW_REGISTERY_SOURCE)- All remaining gating tests transitively affected via
tests/conftest.py
Affected tests to run
| Path | Marker |
|---|---|
utilities/unittests/test_constants.py |
unit |
utilities/unittests/test_oadp.py |
unit |
tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py |
gating |
tests/install_upgrade_operators/product_upgrade/test_upgrade.py |
gating, cnv_upgrade |
tests/install_upgrade_operators/deployment/test_hco_deployment_params.py |
gating, smoke |
tests/install_upgrade_operators/hco_enablement_golden_image_updates/test_custom_golden_images_namespace.py |
gating, smoke |
tests/infrastructure/instance_types/supported_os/test_rhel_os.py |
smoke, gating |
tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py |
smoke |
tests/storage/cdi_upload/test_upload_virtctl.py |
smoke |
tests/storage/cdi_clone/test_clone.py |
smoke, gating |
Real tests (cluster required)
utilities/constants/__init__.py calls _get_cluster_architecture() at module load time to compute Images. Every test session triggers this at collection time.
Collection startup check (confirms no ImportError/AttributeError):
pytest tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py --collect-only
pytest tests/install_upgrade_operators/product_upgrade/test_upgrade.py --collect-only
Renamed constant regression (VIRTCTL_CLI_DOWNLOADS):
pytest tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py -m gating
Typo-fix regression (BREW_REGISTRY_SOURCE):
pytest tests/install_upgrade_operators/product_upgrade/test_upgrade.py -m 'gating and cnv_upgrade'
Contributor
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
COMMIT_SHA=$(gh pr view 5188 --repo RedHatQE/openshift-virtualization-tests --json headRefOid --jq '.headRefOid')
echo "HEAD SHA: $COMMIT_SHA"
# Confirm first changed file and first diff line
gh pr diff 5188 --repo RedHatQE/openshift-virtualization-tests | head -20Length of output: 1257 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - << 'PYEOF'
import json, os, urllib.request, sys
token = os.popen("gh auth token").read().strip()
commit_sha = "9b40449ffdadef0c18f601e9fe536d2dff988459"
body = (
"## Test Execution Plan\n\n"
"**Run smoke tests: True**\n"
"Dependency path: every smoke test (e.g. `tests/virt/node/general/test_container_disk_vm.py`) "
"-> `tests/conftest.py` (`from utilities.constants import ...`) "
"-> `utilities/constants/__init__.py` (restructured package, calls `_get_cluster_architecture()` at import time). "
"`tests/conftest.py` is in the conftest hierarchy of **all** tests in the repo; "
"any import-time failure in `utilities/constants` breaks collection for all smoke tests.\n\n"
"**Run gating tests: True**\n"
"Concrete dependency paths:\n"
"- `tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py` (gating) "
"-> `tests/install_upgrade_operators/relationship_labels/constants.py` imports `VIRTCTL_CLI_DOWNLOADS` "
"from `utilities.constants` (renamed from `VIRTCTL_CLIDOWNLOADS_KUBEVIRT_HYPERCONVERGED`)\n"
"- `tests/install_upgrade_operators/product_upgrade/test_upgrade.py` (gating, cnv_upgrade) "
"-> `tests/install_upgrade_operators/product_upgrade/utils.py` imports `BREW_REGISTRY_SOURCE` "
"(typo fix from `BREW_REGISTERY_SOURCE`)\n"
"- All remaining gating tests transitively affected via `tests/conftest.py`\n\n"
"---\n\n"
"**Affected tests to run**\n\n"
"| Path | Marker |\n"
"|------|--------|\n"
"| `utilities/unittests/test_constants.py` | unit |\n"
"| `utilities/unittests/test_oadp.py` | unit |\n"
"| `tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py` | gating |\n"
"| `tests/install_upgrade_operators/product_upgrade/test_upgrade.py` | gating, cnv_upgrade |\n"
"| `tests/install_upgrade_operators/deployment/test_hco_deployment_params.py` | gating, smoke |\n"
"| `tests/install_upgrade_operators/hco_enablement_golden_image_updates/test_custom_golden_images_namespace.py` | gating, smoke |\n"
"| `tests/infrastructure/instance_types/supported_os/test_rhel_os.py` | smoke, gating |\n"
"| `tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py` | smoke |\n"
"| `tests/storage/cdi_upload/test_upload_virtctl.py` | smoke |\n"
"| `tests/storage/cdi_clone/test_clone.py` | smoke, gating |\n\n"
"---\n\n"
"**Real tests (cluster required)**\n\n"
"`utilities/constants/__init__.py` calls `_get_cluster_architecture()` at **module load time** "
"to compute `Images`. Every test session triggers this at collection time.\n\n"
"Collection startup check (confirms no `ImportError`/`AttributeError`):\n"
"```\n"
"pytest tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py --collect-only\n"
"pytest tests/install_upgrade_operators/product_upgrade/test_upgrade.py --collect-only\n"
"```\n\n"
"Renamed constant regression (`VIRTCTL_CLI_DOWNLOADS`):\n"
"```\n"
"pytest tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py -m gating\n"
"```\n\n"
"Typo-fix regression (`BREW_REGISTRY_SOURCE`):\n"
"```\n"
"pytest tests/install_upgrade_operators/product_upgrade/test_upgrade.py -m 'gating and cnv_upgrade'\n"
"```\n"
)
payload = {
"commit_id": commit_sha,
"event": "COMMENT",
"body": "",
"comments": [
{
"path": "tests/global_config.py",
"line": 18,
"side": "RIGHT",
"body": body
}
]
}
data = json.dumps(payload).encode("utf-8")
req = urllib.request.Request(
"https://api.github.com/repos/RedHatQE/openshift-virtualization-tests/pulls/5188/reviews",
data=data,
headers={
"Authorization": "Bearer " + token,
"Accept": "application/vnd.github+json",
"Content-Type": "application/json",
"X-GitHub-Api-Version": "2022-11-28",
},
method="POST"
)
try:
with urllib.request.urlopen(req) as resp:
result = json.loads(resp.read())
print("SUCCESS: review id=" + str(result.get("id")) + ", url=" + str(result.get("html_url")))
except urllib.error.HTTPError as e:
body_err = e.read().decode()
print("HTTP ERROR " + str(e.code) + ": " + body_err)
sys.exit(1)
PYEOFLength of output: 7207 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 -c "
import json
body = '''## Test Execution Plan
**Run smoke tests: True**
Dependency path: every smoke test (e.g. \`tests/virt/node/general/test_container_disk_vm.py\`) -> \`tests/conftest.py\` (\`from utilities.constants import ...\`) -> \`utilities/constants/__init__.py\` (restructured package, calls \`_get_cluster_architecture()\` at import time). \`tests/conftest.py\` is in the conftest hierarchy of **all** tests in the repo; any import-time failure in \`utilities/constants\` breaks collection for all smoke tests.
**Run gating tests: True**
Concrete dependency paths:
- \`tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py\` (gating) -> \`tests/install_upgrade_operators/relationship_labels/constants.py\` imports \`VIRTCTL_CLI_DOWNLOADS\` from \`utilities.constants\` (renamed from \`VIRTCTL_CLIDOWNLOADS_KUBEVIRT_HYPERCONVERGED\`)
- \`tests/install_upgrade_operators/product_upgrade/test_upgrade.py\` (gating, cnv_upgrade) -> \`tests/install_upgrade_operators/product_upgrade/utils.py\` imports \`BREW_REGISTRY_SOURCE\` (typo fix from \`BREW_REGISTERY_SOURCE\`)
- All remaining gating tests transitively affected via \`tests/conftest.py\`
---
**Affected tests to run**
| Path | Marker |
|------|--------|
| \`utilities/unittests/test_constants.py\` | unit |
| \`utilities/unittests/test_oadp.py\` | unit |
| \`tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py\` | gating |
| \`tests/install_upgrade_operators/product_upgrade/test_upgrade.py\` | gating, cnv_upgrade |
| \`tests/install_upgrade_operators/deployment/test_hco_deployment_params.py\` | gating, smoke |
| \`tests/install_upgrade_operators/hco_enablement_golden_image_updates/test_custom_golden_images_namespace.py\` | gating, smoke |
| \`tests/infrastructure/instance_types/supported_os/test_rhel_os.py\` | smoke, gating |
| \`tests/virt/cluster/common_templates/rhel/test_rhel_os_support.py\` | smoke |
| \`tests/storage/cdi_upload/test_upload_virtctl.py\` | smoke |
| \`tests/storage/cdi_clone/test_clone.py\` | smoke, gating |
---
**Real tests (cluster required)**
\`utilities/constants/__init__.py\` calls \`_get_cluster_architecture()\` at **module load time** to compute \`Images\`. Every test session triggers this at collection time.
Collection startup check (confirms no \`ImportError\`/\`AttributeError\`):
\`\`\`
pytest tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py --collect-only
pytest tests/install_upgrade_operators/product_upgrade/test_upgrade.py --collect-only
\`\`\`
Renamed constant regression (\`VIRTCTL_CLI_DOWNLOADS\`):
\`\`\`
pytest tests/install_upgrade_operators/relationship_labels/test_relationship_labels.py -m gating
\`\`\`
Typo-fix regression (\`BREW_REGISTRY_SOURCE\`):
\`\`\`
pytest tests/install_upgrade_operators/product_upgrade/test_upgrade.py -m 'gating and cnv_upgrade'
\`\`\`'''
payload = {
'commit_id': '9b40449ffdadef0c18f601e9fe536d2dff988459',
'event': 'COMMENT',
'body': '',
'comments': [
{
'path': 'tests/global_config.py',
'line': 18,
'side': 'RIGHT',
'body': body
}
]
}
with open('/tmp/review_payload.json', 'w') as f:
json.dump(payload, f)
print('payload written')
"
gh api \
--method POST \
-H "Accept: application/vnd.github+json" \
-H "X-GitHub-Api-Version: 2022-11-28" \
/repos/RedHatQE/openshift-virtualization-tests/pulls/5188/reviews \
--input /tmp/review_payload.json \
--jq '"Review posted: id=\(.id) url=\(.html_url)"'Length of output: 3789 The Test Execution Plan has been posted as a non-blocking inline review comment on |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Replace the monolithic
utilities/constants.pywith autilities/constants/package of thematic submodules. A temporary__init__.pycompat shim re-exports every public name so all existingfrom utilities.constants import Xstatements continue to work unchanged while per-team import migrations land in follow-up PRs.Submodules:
aaq, components, hco, images, instance_types, misc, monitoring, namespaces, networking, os_matrix, storage, timeouts, virt.Bundled cleanup:
TIMEOUT_35MIN, TIMEOUT_40SEC, TIMEOUT_90MIN, TIMEOUT_12HRS, NONE_STR, KUBEVIRT_VIRT_OPERATOR_UP, HOSTPATH_CSI_BASIC)BREW_REGISTERY_SOURCE->BREW_REGISTRY_SOURCE(3 files)VIRTCTL_CLIDOWNLOADS_KUBEVIRT_HYPERCONVERGED->VIRTCTL_CLI_DOWNLOADSGenerated-by: Cursor Agent (Claude Sonnet 4.6)
Which issue(s) this PR fixes:
Part of effort to refactor/clean-up utilities modules to remove all circular imports and create a more clear structure.
Special notes for reviewer:
This is a first PR in a series.
https://redhat.atlassian.net/browse/CNV-80952
__init__.pyis a temp to not break existing imports undertests/. Follow-up PRs will be updating imports to new submodules in every sig, and dropping__init__.pyin the endjira-ticket:
https://redhat.atlassian.net/browse/CNV-89562
Summary by CodeRabbit