feat: Add ScaledObject resources #2497
Conversation
WalkthroughAdds ApiGroup.KEDA_SH constant and a new NamespacedResource Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the "Integrations" page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
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
💡 Tips
For more information, please refer to the project documentation or contact the maintainers. |
|
/verified |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
ocp_resources/scaled_object.py (2)
16-29: Tighten the type of triggers for clarity and correctness.
triggersare objects with well-defined fields (e.g., type/metadata/authRef). Usinglist[dict[str, Any]]improves readability and static analysis without being overly strict.Apply this diff to refine the annotation:
- triggers: list[Any] | None = None, + triggers: list[dict[str, Any]] | None = None,
30-52: Docstring nit: minor grammar polish."AdvancedConfig specifies advance scaling options" → "AdvancedConfig specifies advanced scaling options".
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
class_generator/schema/__resources-mappings.json.gzis excluded by!**/*.gz,!class_generator/schema/**class_generator/schema/_definitions.jsonis excluded by!class_generator/schema/**
📒 Files selected for processing (2)
ocp_resources/resource.py(1 hunks)ocp_resources/scaled_object.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
ocp_resources/scaled_object.py (2)
ocp_resources/resource.py (4)
NamespacedResource(1533-1643)ApiGroup(466-573)to_dict(734-738)to_dict(1641-1643)ocp_resources/exceptions.py (1)
MissingRequiredArgumentError(5-10)
🔇 Additional comments (2)
ocp_resources/resource.py (1)
508-508: ApiGroup.KEDA_SH constant looks correct and unblocker for KEDA resources.The value "keda.sh" matches KEDA's API group and will allow resolving apiVersion dynamically for ScaledObject.
ocp_resources/scaled_object.py (1)
9-15: LGTM: Correct API group for KEDA ScaledObject.Setting
api_grouptoNamespacedResource.ApiGroup.KEDA_SHis accurate and aligns with dynamic apiVersion resolution in the base class.
|
@dbasunag on which cluster version did you update the schema? (k8s version - from oc version) |
|
@myakove this is the cluster information But this cluster is borrowed from the team the requires this resource. |
|
/verified |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
class_generator/tests/test_class_generator.py (3)
94-109: Consider adding ScaledObject to generator coverage (optional)Given this PR introduces the ScaledObject resource, consider adding "ScaledObject" here after you commit its golden(s), to keep generator coverage aligned with newly supported resources.
Apply this update when the golden exists:
resource_kinds = [ "Pod", "Pipeline", "OAuth", "ClusterOperator", "ImageContentSourcePolicy", "ServiceMeshMember", "NMState", "Deployment", "Machine", "APIServer", "Secret", "ConfigMap", "DNS", + "ScaledObject", "ServingRuntime", "RouteAdvertisements", ]
44-45: Use content-based comparison to avoid shallow stat-equality edge casesfilecmp.cmp defaults to shallow=True; using shallow=False makes the check robust against rare stat-based false positives.
Apply:
- if not filecmp.cmp(output_file, expected_file): + if not filecmp.cmp(output_file, expected_file, shallow=False):
114-115: Right-size the thread pool to the workload (optional)Scaling max_workers to the actual list size avoids spinning up surplus threads when resource_kinds is small.
- with ThreadPoolExecutor(max_workers=8) as executor: + workers = min(8, len(resource_kinds)) + with ThreadPoolExecutor(max_workers=workers) as executor:
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
class_generator/schema/__resources-mappings.json.gzis excluded by!**/*.gz,!class_generator/schema/**class_generator/schema/_definitions.jsonis excluded by!class_generator/schema/**
📒 Files selected for processing (5)
class_generator/tests/manifests/Deployment/deployment.py(4 hunks)class_generator/tests/manifests/Pod/pod.py(5 hunks)class_generator/tests/test_class_generator.py(1 hunks)ocp_resources/resource.py(1 hunks)ocp_resources/scaled_object.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- ocp_resources/scaled_object.py
- ocp_resources/resource.py
⏰ 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). (9)
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: can-be-merged
- GitHub Check: python-module-install
- GitHub Check: conventional-title
- GitHub Check: tox
🔇 Additional comments (2)
class_generator/tests/test_class_generator.py (1)
94-109: Golden manifests for Pipeline and OAuth found — no test break expectedVerified that matching manifest directories and files exist:
- class_generator/tests/manifests/Pipeline/pipeline.py
- class_generator/tests/manifests/OAuth/oauth.py
No further action required.
class_generator/tests/manifests/Deployment/deployment.py (1)
96-99: Docstring refresh aligns with current oc explain (v1.31) — LGTMUpdates livenessProbe placement and scheduling/security fields organization in line with modern schema. No functional/API changes. Looks good.
Also applies to: 134-137, 172-172, 202-209
Short description:
More details:
What this PR does / why we need it:
Which issue(s) this PR fixes:
Special notes for reviewer:
Bug:
Summary by CodeRabbit
New Features
Documentation
Chores
Tests