net, tests, hot-plug: Avoid redundant VMI interface lookup - #5578
Conversation
|
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)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
📝 WalkthroughWalkthrough
ChangesTyped ResourceField refactor
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
|
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
Branch Management
Label Management
✅ Merge RequirementsThis PR will be automatically approved when the following conditions are met:
📊 Review ProcessApprovers and ReviewersApprovers:
Reviewers:
Available Labels
AI Features
Security Checks
💡 Tips
📌 Additional InformationCustom Commands:
For more information, please refer to the project documentation or contact the maintainers. |
![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)
tests/network/l2_bridge/libl2bridge.py (1)
245-250: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove the unused
get_guest_vm_interface_name_by_vmi_interface_namehelperNo callers remain, so this should be deleted to avoid dead code and keep the module’s API surface accurate.
🤖 Prompt for 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. In `@tests/network/l2_bridge/libl2bridge.py` around lines 245 - 250, The helper get_guest_vm_interface_name_by_vmi_interface_name is no longer used anywhere, so remove the entire function from the l2_bridge module to eliminate dead code and keep the public API accurate. Make sure there are no remaining references to this helper elsewhere in the module before deleting it.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@tests/network/l2_bridge/libl2bridge.py`:
- Around line 245-250: The helper
get_guest_vm_interface_name_by_vmi_interface_name is no longer used anywhere, so
remove the entire function from the l2_bridge module to eliminate dead code and
keep the public API accurate. Make sure there are no remaining references to
this helper elsewhere in the module before deleting it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ddf27c31-07bb-43dd-9b13-9054642cb461
📒 Files selected for processing (2)
tests/network/l2_bridge/libl2bridge.pytests/network/l2_bridge/test_bridge_nic_hot_plug.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
RedHatQE/openshift-virtualization-tests-design-docs(manual)
6eb49c4 to
13e85af
Compare
|
/retest verify-bugs-are-open |
openshift-virtualization-qe-bot
added
tox:verify-tc-requirement-polarion:running
|
Clean rebase detected — no code changes compared to previous head ( |
|
/retest verify-bugs-are-open |
|
/approve |
|
/check-can-merge |
|
New container for quay.io/openshift-cnv/openshift-virtualization-tests:latest published |
|
/cherry-pick cnv-4.22 cnv-4.21 cnv-4.20 cnv-4.19 cnv-4.18 |
|
Manual cherry-pick is needed |
|
/cherry-pick cnv-4.22 |
|
Manual cherry-pick is needed |
|
Cherry-pick conflicts were resolved by AI Cherry-picked PR net, tests, hot-plug: Avoid redundant VMI interface lookup into cnv-4.21: #5956 @yossisegev Manual verification is required — please review the changes and test before merging. |
|
Cherry-pick conflicts were resolved by AI Cherry-picked PR net, tests, hot-plug: Avoid redundant VMI interface lookup into cnv-4.22: #5955 @yossisegev Manual verification is required — please review the changes and test before merging. |
|
Cherry-pick conflicts were resolved by AI Cherry-picked PR net, tests, hot-plug: Avoid redundant VMI interface lookup into cnv-4.20: #5957 @yossisegev Manual verification is required — please review the changes and test before merging. |
12 participants
hot_plug_interface() already returns the full interface status including interfaceName. Pass it through to set_secondary_static_ip_address() instead of re-querying VMI status for the same value.
This PR replaces #5419, which required fixes, but its original author is away and I cannot push to their fork (in order to apply these fixes).
Assisted-by: Claude noreply@anthropic.com
Assisted-by: Yossi Segev ysegev@redhat.com
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
jira-ticket:
https://redhat.atlassian.net/browse/CNV-88390
Summary by CodeRabbit