Disable weighers in vmware-hana-bin-packing pipeline#816
Merged
Conversation
umswmayj
requested review from
PhilippMatthes,
SoWieMarkus,
auhlig,
juliusclausnitzer and
mblos
as code owners
📝 WalkthroughWalkthroughThe VMware HANA pipeline definition in the Cortex Nova Helm bundle is updated by removing all weighers from the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 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.
🧹 Nitpick comments (1)
helm/bundles/cortex-nova/templates/pipelines.yaml (1)
67-78: ⚡ Quick winUpdate the pipeline description to reflect the disabled weighers.
The
descriptionstill references active HANA workload scheduling, but with bothfilters: []andweighers: [], this pipeline is now a complete pass-through — Cortex returns Nova's host list unmodified. The description is misleading as-is.Consider adding a note explaining why weighers are disabled (e.g., temporary disablement, debugging, migration), so future readers and operators understand the intent without having to dig through git history.
✏️ Suggested description update
description: | Nova provides virtual machine placement on compute hosts for OpenStack. After applying its own filtering and weighing logic, it delegates to cortex for additional filtering and weighing via this external scheduler pipeline. Cortex returns a ranked list of hosts back to nova for final selection. This is the pipeline used for VMware. Specifically, this pipeline is used for HANA workloads. + + NOTE: Weighers are currently disabled (weighers: []) — this pipeline acts + as a pass-through. Re-enable weighers once the disablement reason is resolved.🤖 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 `@helm/bundles/cortex-nova/templates/pipelines.yaml` around lines 67 - 78, The pipeline description is misleading because filters: [] and weighers: [] make this a pass-through; update the description field in pipelines.yaml (the block with type: filter-weigher) to explicitly state that weighers are currently disabled and the pipeline returns Nova's host list unmodified, and include a short rationale (e.g., temporary disablement for debugging/migration) and an optional TODO or link to the issue/PR so future readers know intent without digging through git history.
🤖 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.
Nitpick comments:
In `@helm/bundles/cortex-nova/templates/pipelines.yaml`:
- Around line 67-78: The pipeline description is misleading because filters: []
and weighers: [] make this a pass-through; update the description field in
pipelines.yaml (the block with type: filter-weigher) to explicitly state that
weighers are currently disabled and the pipeline returns Nova's host list
unmodified, and include a short rationale (e.g., temporary disablement for
debugging/migration) and an optional TODO or link to the issue/PR so future
readers know intent without digging through git history.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dc22c9c8-868d-4cbb-bb96-a9ff6e46ddfb
📒 Files selected for processing (1)
helm/bundles/cortex-nova/templates/pipelines.yaml
Contributor
Test Coverage ReportTest Coverage 📊: 69.1% |
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
1 participant
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.
No description provided.