feat(aqc): add max reclaimed resource for memory and cpu#205
feat(aqc): add max reclaimed resource for memory and cpu#205JustinChengLZ wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdmin QoS configuration adds optional CPU and memory reclaim ratio fields to the API types and CRD schema. The generated deepcopy code now copies both new pointer fields. ChangesAdmin QoS reclaim ratio fields
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
JustinChengLZ
force-pushed
the
dev/reclaim-resource-max
branch
from
1039b1c to
10f1404
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
pkg/apis/config/v1alpha1/adminqos.go (2)
254-259: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueComment ordering differs from sibling fields.
Validation markers precede the description here, whereas other fields (e.g.
FreeBasedRatioat Line 284) place the description first. Functionally harmless since controller-gen still produced the correct CRD description, but worth aligning for consistency.🤖 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 `@pkg/apis/config/v1alpha1/adminqos.go` around lines 254 - 259, The comment order for ReclaimedCPUMaxRatio is inconsistent with sibling fields like FreeBasedRatio in AdminQOSSpec. Reorder the kubebuilder validation markers and field description so the human-readable description comes first, followed by the validation tags and +optional, matching the style used by the other fields in this struct.
254-259: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueJSON tag drops the "Ratio" suffix present in the Go field name.
ReclaimedCPUMaxRatio/ReclaimedMemoryMaxRatiomap toreclaimedCPUMax/reclaimedMemoryMax, while sibling fields keep full name correspondence (e.g.FreeBasedRatio→freeBasedRatio,CacheBasedRatio→cacheBasedRatio). Consider aligning the JSON key naming convention for consistency, though this is a cosmetic API surface choice rather than a defect.Also applies to: 313-318
🤖 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 `@pkg/apis/config/v1alpha1/adminqos.go` around lines 254 - 259, Align the JSON field names for the AdminQoS ratio fields so they consistently preserve the “Ratio” suffix like the other ratio properties. Update the ReclaimedCPUMaxRatio and ReclaimedMemoryMaxRatio struct tags in adminqos.go to use matching JSON keys, and verify the same naming convention is applied wherever these fields are defined or referenced, including the duplicate declaration noted by the review.
🤖 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 `@pkg/apis/config/v1alpha1/adminqos.go`:
- Around line 254-259: The comment order for ReclaimedCPUMaxRatio is
inconsistent with sibling fields like FreeBasedRatio in AdminQOSSpec. Reorder
the kubebuilder validation markers and field description so the human-readable
description comes first, followed by the validation tags and +optional, matching
the style used by the other fields in this struct.
- Around line 254-259: Align the JSON field names for the AdminQoS ratio fields
so they consistently preserve the “Ratio” suffix like the other ratio
properties. Update the ReclaimedCPUMaxRatio and ReclaimedMemoryMaxRatio struct
tags in adminqos.go to use matching JSON keys, and verify the same naming
convention is applied wherever these fields are defined or referenced, including
the duplicate declaration noted by the review.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: fe4f41c9-97f9-4611-a1cd-a35fad60141c
📒 Files selected for processing (3)
config/crd/bases/config.katalyst.kubewharf.io_adminqosconfigurations.yamlpkg/apis/config/v1alpha1/adminqos.gopkg/apis/config/v1alpha1/zz_generated.deepcopy.go
✅ Files skipped from review due to trivial changes (1)
- pkg/apis/config/v1alpha1/zz_generated.deepcopy.go
JustinChengLZ
force-pushed
the
dev/reclaim-resource-max
branch
from
10f1404 to
78a7ab0
Compare
1 participant
What type of PR is this?
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for your reviewer:
Summary by CodeRabbit