-
Notifications
You must be signed in to change notification settings - Fork 283
Playbook generator #1136
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Playbook generator #1136
Conversation
* update kube-prometheus-stack to 47.2.0
* Single page playbook generator, simple navigation * workaround streamlit bug --------- Co-authored-by: Pavan Gudiwada <[email protected]> Co-authored-by: Robusta Runner <[email protected]>
Operation enum in on_ingress_all_changes was not converted to yaml properly in output
E.g. on_ingress_create didn't work because "create" couldn't be converted by streamlit pydantic to K8sOperationType.CREATE. If K8sOperationType is a StrEnum and not an Enum, it works.
WalkthroughThis update introduces a Streamlit-based Playbook Generator UI for Robusta, including new scripts, Dockerfile, and supporting code refactors. It adds Python 3.10 support, updates dependencies, and enhances trigger/action modeling with Pydantic and enums. Several classes are simplified for maintainability, and new utilities for discovering playbook actions and mapping triggers to actions are provided. Changes
Sequence Diagram(s)Playbook Generator UI FlowsequenceDiagram
participant User
participant StreamlitApp
participant ExamplesGenerator
participant SessionState
User->>StreamlitApp: Open app
StreamlitApp->>SessionState: Initialize or read current page/screen
alt Demo Playbooks
StreamlitApp->>ExamplesGenerator: Load demo playbooks
StreamlitApp->>User: Display demo playbooks
User->>StreamlitApp: Select demo playbook
StreamlitApp->>SessionState: Update selected playbook, switch to builder
else Playbook Builder
StreamlitApp->>ExamplesGenerator: Load triggers/actions
StreamlitApp->>User: Step 1: Select trigger + params
User->>StreamlitApp: Submit trigger form
StreamlitApp->>SessionState: Store trigger data, go to next step
StreamlitApp->>User: Step 2: Select action + params
User->>StreamlitApp: Submit action form
StreamlitApp->>SessionState: Store action data, go to next step
StreamlitApp->>User: Step 3: Show generated YAML
User->>StreamlitApp: Copy/apply YAML or restart
end
Trigger Class Refactor (Generic Example)sequenceDiagram
participant Developer
participant TriggerClass
Developer->>TriggerClass: Define class with class-level Literal attributes
Note right of TriggerClass: No explicit constructor needed
Developer->>TriggerClass: Use kind and operation as class constants
TriggerClass-->>Developer: Provides fixed trigger type/operation
Playbook Action DiscoverysequenceDiagram
participant Script
participant FileSystem
participant ModuleImporter
participant ActionClass
Script->>FileSystem: List Python files in directory
loop For each file
Script->>ModuleImporter: Import module
ModuleImporter->>ActionClass: Inspect members
ActionClass-->>Script: Identify playbook actions
Script->>ActionClass: Wrap as Action objects
end
Script-->>Script: Return list of Action objects
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 10
🧹 Nitpick comments (7)
streamlit.Dockerfile (3)
18-18: Consider using .dockerignore to exclude unnecessary files.Copying the entire codebase with
COPY . /robusta/might include unnecessary files like.git,__pycache__, etc. Consider creating a.dockerignorefile to exclude development files and improve build efficiency.
22-22: Consider installing only required extras for production.Installing all extras with
--extras "all"might include unnecessary dependencies for the Streamlit app. Consider specifying only the required extras for the playbook generator functionality.
1-26: Consider adding non-root user for security.The container runs as root, which is a security concern. Consider adding a non-root user for running the Streamlit application.
+# Create non-root user +RUN groupadd -r streamlit && useradd -r -g streamlit streamlit +RUN chown -R streamlit:streamlit /robusta +USER streamlit + # Command to run Streamlit when the container starts CMD ["streamlit", "run", "./scripts/playbook_generator.py"]scripts/main_app.py (1)
1-1: Consider adding error handling for missing modules.The import statement assumes the
pagesmodule and its submodules exist. Consider adding error handling to gracefully handle missing modules during development.+try: from pages import demo_playbooks, playbook_builder +except ImportError as e: + st.error(f"Failed to import required modules: {e}") + st.stop()src/robusta/integrations/prometheus/trigger.py (1)
65-70: Consider adding a default value for k8s_providers field.While the Field descriptions improve documentation, the
k8s_providersfield is missing a default value unlike other optional fields. For consistency and to avoid potential issues, consider adding a default.- k8s_providers: Optional[List[str]] = Field(description="Optional. Only fire this trigger on clusters from a certain provider (e.g. EKS)") + k8s_providers: Optional[List[str]] = Field(default=None, description="Optional. Only fire this trigger on clusters from a certain provider (e.g. EKS)")scripts/generate_playbook_descriptions.py (1)
37-68: Clean up debugging prints and commented code.The function contains debugging print statements and a large block of commented code. Consider removing or properly documenting why this code is preserved.
def find_playbook_actions(scripts_root): python_files = glob.glob(f"{scripts_root}/*.py") actions = [] for script in python_files: - print(f"found playbook file: {script}") filename = os.path.basename(script) (module_name, ext) = os.path.splitext(filename) spec = importlib.util.spec_from_file_location(module_name, script) module = importlib.util.module_from_spec(spec) spec.loader.exec_module(module) playbooks = inspect.getmembers( module, lambda f: Action.is_action(f), ) for _, func in playbooks: - print("found playbook", func) action = Action(func) actions.append(action) - - #description = PlaybookDescription( - # function_name=func.__name__, - # builtin_trigger_params=func.__playbook["default_trigger_params"], - # docs=inspect.getdoc(func), - # src=inspect.getsource(func), - # src_file=inspect.getsourcefile(func), - # action_params=get_params_schema(func), - #) - #print(description.json(), "\n\n") return actionsAlternatively, if the commented code needs to be preserved for future reference, add a TODO comment explaining why:
+ # TODO: The PlaybookDescription generation code below is preserved for future reference + # when we need to generate detailed playbook documentation #description = PlaybookDescription(scripts/playbook_generator.py (1)
36-36: Simplify dictionary key checking.Use more Pythonic syntax for checking dictionary keys.
Apply this diff to simplify the code:
-for key in st.session_state.keys(): +for key in st.session_state:Also applies to: 43-43
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
.python-version(1 hunks)pyproject.toml(2 hunks)scripts/custom_streamlit_pydantic.py(1 hunks)scripts/generate_kubernetes_code.py(4 hunks)scripts/generate_playbook_descriptions.py(3 hunks)scripts/main_app.py(1 hunks)scripts/playbook_generator.py(1 hunks)src/robusta/core/model/base_params.py(1 hunks)src/robusta/core/model/k8s_operation_type.py(1 hunks)src/robusta/core/playbooks/generation.py(3 hunks)src/robusta/core/triggers/error_event_trigger.py(3 hunks)src/robusta/core/triggers/helm_releases_triggers.py(6 hunks)src/robusta/integrations/kubernetes/autogenerated/events.py(2 hunks)src/robusta/integrations/kubernetes/autogenerated/models.py(1 hunks)src/robusta/integrations/kubernetes/autogenerated/triggers.py(20 hunks)src/robusta/integrations/kubernetes/autogenerated/v1/models.py(2 hunks)src/robusta/integrations/kubernetes/base_triggers.py(2 hunks)src/robusta/integrations/prometheus/trigger.py(2 hunks)streamlit.Dockerfile(1 hunks)
🧰 Additional context used
🪛 Ruff (0.11.9)
src/robusta/integrations/kubernetes/autogenerated/v1/models.py
26-26: Ingress may be undefined, or defined from star imports
(F405)
src/robusta/core/playbooks/generation.py
14-14: argparse imported but unused
Remove unused import: argparse
(F401)
19-19: Redefinition of unused Callable from line 4
Remove definition: Callable
(F811)
21-21: pydantic.BaseModel imported but unused
Remove unused import: pydantic.BaseModel
(F401)
22-22: Redefinition of unused Action from line 9
(F811)
scripts/generate_playbook_descriptions.py
77-77: Local variable triggers is assigned to but never used
Remove assignment to unused variable triggers
(F841)
src/robusta/integrations/kubernetes/autogenerated/triggers.py
3-3: typing.Dict imported but unused
Remove unused import: typing.Dict
(F401)
8-8: from robusta.integrations.kubernetes.autogenerated.events import * used; unable to detect undefined names
(F403)
19-19: PodChangeEvent may be undefined, or defined from star imports
(F405)
28-28: PodChangeEvent may be undefined, or defined from star imports
(F405)
37-37: PodChangeEvent may be undefined, or defined from star imports
(F405)
56-56: ReplicaSetChangeEvent may be undefined, or defined from star imports
(F405)
65-65: ReplicaSetChangeEvent may be undefined, or defined from star imports
(F405)
74-74: ReplicaSetChangeEvent may be undefined, or defined from star imports
(F405)
93-93: DaemonSetChangeEvent may be undefined, or defined from star imports
(F405)
102-102: DaemonSetChangeEvent may be undefined, or defined from star imports
(F405)
111-111: DaemonSetChangeEvent may be undefined, or defined from star imports
(F405)
130-130: DeploymentChangeEvent may be undefined, or defined from star imports
(F405)
139-139: DeploymentChangeEvent may be undefined, or defined from star imports
(F405)
148-148: DeploymentChangeEvent may be undefined, or defined from star imports
(F405)
167-167: StatefulSetChangeEvent may be undefined, or defined from star imports
(F405)
176-176: StatefulSetChangeEvent may be undefined, or defined from star imports
(F405)
185-185: StatefulSetChangeEvent may be undefined, or defined from star imports
(F405)
204-204: ServiceChangeEvent may be undefined, or defined from star imports
(F405)
213-213: ServiceChangeEvent may be undefined, or defined from star imports
(F405)
222-222: ServiceChangeEvent may be undefined, or defined from star imports
(F405)
241-241: EventChangeEvent may be undefined, or defined from star imports
(F405)
250-250: EventChangeEvent may be undefined, or defined from star imports
(F405)
259-259: EventChangeEvent may be undefined, or defined from star imports
(F405)
278-278: HorizontalPodAutoscalerChangeEvent may be undefined, or defined from star imports
(F405)
287-287: HorizontalPodAutoscalerChangeEvent may be undefined, or defined from star imports
(F405)
296-296: HorizontalPodAutoscalerChangeEvent may be undefined, or defined from star imports
(F405)
315-315: NodeChangeEvent may be undefined, or defined from star imports
(F405)
324-324: NodeChangeEvent may be undefined, or defined from star imports
(F405)
333-333: NodeChangeEvent may be undefined, or defined from star imports
(F405)
352-352: ClusterRoleChangeEvent may be undefined, or defined from star imports
(F405)
361-361: ClusterRoleChangeEvent may be undefined, or defined from star imports
(F405)
370-370: ClusterRoleChangeEvent may be undefined, or defined from star imports
(F405)
389-389: ClusterRoleBindingChangeEvent may be undefined, or defined from star imports
(F405)
398-398: ClusterRoleBindingChangeEvent may be undefined, or defined from star imports
(F405)
407-407: ClusterRoleBindingChangeEvent may be undefined, or defined from star imports
(F405)
426-426: JobChangeEvent may be undefined, or defined from star imports
(F405)
435-435: JobChangeEvent may be undefined, or defined from star imports
(F405)
444-444: JobChangeEvent may be undefined, or defined from star imports
(F405)
463-463: NamespaceChangeEvent may be undefined, or defined from star imports
(F405)
472-472: NamespaceChangeEvent may be undefined, or defined from star imports
(F405)
481-481: NamespaceChangeEvent may be undefined, or defined from star imports
(F405)
500-500: ServiceAccountChangeEvent may be undefined, or defined from star imports
(F405)
509-509: ServiceAccountChangeEvent may be undefined, or defined from star imports
(F405)
518-518: ServiceAccountChangeEvent may be undefined, or defined from star imports
(F405)
537-537: PersistentVolumeChangeEvent may be undefined, or defined from star imports
(F405)
546-546: PersistentVolumeChangeEvent may be undefined, or defined from star imports
(F405)
555-555: PersistentVolumeChangeEvent may be undefined, or defined from star imports
(F405)
574-574: PersistentVolumeClaimChangeEvent may be undefined, or defined from star imports
(F405)
583-583: PersistentVolumeClaimChangeEvent may be undefined, or defined from star imports
(F405)
592-592: PersistentVolumeClaimChangeEvent may be undefined, or defined from star imports
(F405)
611-611: NetworkPolicyChangeEvent may be undefined, or defined from star imports
(F405)
620-620: NetworkPolicyChangeEvent may be undefined, or defined from star imports
(F405)
629-629: NetworkPolicyChangeEvent may be undefined, or defined from star imports
(F405)
648-648: ConfigMapChangeEvent may be undefined, or defined from star imports
(F405)
657-657: ConfigMapChangeEvent may be undefined, or defined from star imports
(F405)
666-666: ConfigMapChangeEvent may be undefined, or defined from star imports
(F405)
685-685: IngressChangeEvent may be undefined, or defined from star imports
(F405)
694-694: IngressChangeEvent may be undefined, or defined from star imports
(F405)
703-703: IngressChangeEvent may be undefined, or defined from star imports
(F405)
722-722: KubernetesAnyChangeEvent may be undefined, or defined from star imports
(F405)
731-731: KubernetesAnyChangeEvent may be undefined, or defined from star imports
(F405)
740-740: KubernetesAnyChangeEvent may be undefined, or defined from star imports
(F405)
scripts/playbook_generator.py
8-8: typing.List imported but unused
Remove unused import
(F401)
8-8: typing.Set imported but unused
Remove unused import
(F401)
8-8: typing.Optional imported but unused
Remove unused import
(F401)
8-8: typing.Literal imported but unused
Remove unused import
(F401)
9-9: enum.Enum imported but unused
Remove unused import: enum.Enum
(F401)
14-14: streamlit_pydantic imported but unused
Remove unused import: streamlit_pydantic
(F401)
17-17: pydantic.BaseModel imported but unused
Remove unused import: pydantic.BaseModel
(F401)
20-20: custom_streamlit_pydantic.GroupOptionalFieldsStrategy imported but unused
Remove unused import: custom_streamlit_pydantic.GroupOptionalFieldsStrategy
(F401)
36-36: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
43-43: Use key in dict instead of key in dict.keys()
Remove .keys()
(SIM118)
183-183: Local variable action_names is assigned to but never used
Remove assignment to unused variable action_names
(F841)
208-208: Undefined name action_data
(F821)
210-210: Do not use bare except
(E722)
251-251: Local variable action_data is assigned to but never used
Remove assignment to unused variable action_data
(F841)
src/robusta/core/triggers/error_event_trigger.py
3-3: enum.Enum imported but unused
Remove unused import: enum.Enum
(F401)
src/robusta/core/triggers/helm_releases_triggers.py
3-3: logging imported but unused
Remove unused import: logging
(F401)
4-4: sys imported but unused
Remove unused import: sys
(F401)
🪛 Pylint (3.3.7)
src/robusta/integrations/kubernetes/autogenerated/v1/models.py
[error] 26-26: Undefined variable 'Ingress'
(E0602)
scripts/custom_streamlit_pydantic.py
[refactor] 13-13: Too many arguments (9/5)
(R0913)
[refactor] 13-13: Too many positional arguments (9/5)
(R0917)
[refactor] 13-13: Too many local variables (17/15)
(R0914)
src/robusta/integrations/kubernetes/autogenerated/triggers.py
[error] 19-19: Undefined variable 'PodChangeEvent'
(E0602)
[refactor] 13-13: Too few public methods (1/2)
(R0903)
[error] 28-28: Undefined variable 'PodChangeEvent'
(E0602)
[refactor] 22-22: Too few public methods (1/2)
(R0903)
[error] 37-37: Undefined variable 'PodChangeEvent'
(E0602)
[refactor] 31-31: Too few public methods (1/2)
(R0903)
[refactor] 40-40: Too few public methods (1/2)
(R0903)
[error] 56-56: Undefined variable 'ReplicaSetChangeEvent'
(E0602)
[error] 65-65: Undefined variable 'ReplicaSetChangeEvent'
(E0602)
[refactor] 59-59: Too few public methods (1/2)
(R0903)
[error] 74-74: Undefined variable 'ReplicaSetChangeEvent'
(E0602)
[refactor] 68-68: Too few public methods (1/2)
(R0903)
[refactor] 77-77: Too few public methods (1/2)
(R0903)
[error] 93-93: Undefined variable 'DaemonSetChangeEvent'
(E0602)
[error] 102-102: Undefined variable 'DaemonSetChangeEvent'
(E0602)
[refactor] 96-96: Too few public methods (1/2)
(R0903)
[error] 111-111: Undefined variable 'DaemonSetChangeEvent'
(E0602)
[refactor] 105-105: Too few public methods (1/2)
(R0903)
[refactor] 114-114: Too few public methods (1/2)
(R0903)
[error] 130-130: Undefined variable 'DeploymentChangeEvent'
(E0602)
[error] 139-139: Undefined variable 'DeploymentChangeEvent'
(E0602)
[refactor] 133-133: Too few public methods (1/2)
(R0903)
[error] 148-148: Undefined variable 'DeploymentChangeEvent'
(E0602)
[refactor] 142-142: Too few public methods (1/2)
(R0903)
[refactor] 151-151: Too few public methods (1/2)
(R0903)
[error] 167-167: Undefined variable 'StatefulSetChangeEvent'
(E0602)
[error] 176-176: Undefined variable 'StatefulSetChangeEvent'
(E0602)
[refactor] 170-170: Too few public methods (1/2)
(R0903)
[error] 185-185: Undefined variable 'StatefulSetChangeEvent'
(E0602)
[refactor] 179-179: Too few public methods (1/2)
(R0903)
[refactor] 188-188: Too few public methods (1/2)
(R0903)
[error] 204-204: Undefined variable 'ServiceChangeEvent'
(E0602)
[error] 213-213: Undefined variable 'ServiceChangeEvent'
(E0602)
[refactor] 207-207: Too few public methods (1/2)
(R0903)
[error] 222-222: Undefined variable 'ServiceChangeEvent'
(E0602)
[refactor] 216-216: Too few public methods (1/2)
(R0903)
[refactor] 225-225: Too few public methods (1/2)
(R0903)
[error] 241-241: Undefined variable 'EventChangeEvent'
(E0602)
[error] 250-250: Undefined variable 'EventChangeEvent'
(E0602)
[refactor] 244-244: Too few public methods (1/2)
(R0903)
[error] 259-259: Undefined variable 'EventChangeEvent'
(E0602)
[refactor] 253-253: Too few public methods (1/2)
(R0903)
[refactor] 262-262: Too few public methods (1/2)
(R0903)
[error] 278-278: Undefined variable 'HorizontalPodAutoscalerChangeEvent'
(E0602)
[error] 287-287: Undefined variable 'HorizontalPodAutoscalerChangeEvent'
(E0602)
[refactor] 281-281: Too few public methods (1/2)
(R0903)
[error] 296-296: Undefined variable 'HorizontalPodAutoscalerChangeEvent'
(E0602)
[refactor] 290-290: Too few public methods (1/2)
(R0903)
[refactor] 299-299: Too few public methods (1/2)
(R0903)
[error] 315-315: Undefined variable 'NodeChangeEvent'
(E0602)
[error] 324-324: Undefined variable 'NodeChangeEvent'
(E0602)
[refactor] 318-318: Too few public methods (1/2)
(R0903)
[error] 333-333: Undefined variable 'NodeChangeEvent'
(E0602)
[refactor] 327-327: Too few public methods (1/2)
(R0903)
[refactor] 336-336: Too few public methods (1/2)
(R0903)
[error] 352-352: Undefined variable 'ClusterRoleChangeEvent'
(E0602)
[error] 361-361: Undefined variable 'ClusterRoleChangeEvent'
(E0602)
[refactor] 355-355: Too few public methods (1/2)
(R0903)
[error] 370-370: Undefined variable 'ClusterRoleChangeEvent'
(E0602)
[refactor] 364-364: Too few public methods (1/2)
(R0903)
[refactor] 373-373: Too few public methods (1/2)
(R0903)
[error] 389-389: Undefined variable 'ClusterRoleBindingChangeEvent'
(E0602)
[error] 398-398: Undefined variable 'ClusterRoleBindingChangeEvent'
(E0602)
[refactor] 392-392: Too few public methods (1/2)
(R0903)
[error] 407-407: Undefined variable 'ClusterRoleBindingChangeEvent'
(E0602)
[refactor] 401-401: Too few public methods (1/2)
(R0903)
[refactor] 410-410: Too few public methods (1/2)
(R0903)
[error] 426-426: Undefined variable 'JobChangeEvent'
(E0602)
[error] 435-435: Undefined variable 'JobChangeEvent'
(E0602)
[refactor] 429-429: Too few public methods (1/2)
(R0903)
[error] 444-444: Undefined variable 'JobChangeEvent'
(E0602)
[refactor] 438-438: Too few public methods (1/2)
(R0903)
[refactor] 447-447: Too few public methods (1/2)
(R0903)
[error] 463-463: Undefined variable 'NamespaceChangeEvent'
(E0602)
[error] 472-472: Undefined variable 'NamespaceChangeEvent'
(E0602)
[refactor] 466-466: Too few public methods (1/2)
(R0903)
[error] 481-481: Undefined variable 'NamespaceChangeEvent'
(E0602)
[refactor] 475-475: Too few public methods (1/2)
(R0903)
[refactor] 484-484: Too few public methods (1/2)
(R0903)
[error] 500-500: Undefined variable 'ServiceAccountChangeEvent'
(E0602)
[error] 509-509: Undefined variable 'ServiceAccountChangeEvent'
(E0602)
[refactor] 503-503: Too few public methods (1/2)
(R0903)
[error] 518-518: Undefined variable 'ServiceAccountChangeEvent'
(E0602)
[refactor] 512-512: Too few public methods (1/2)
(R0903)
[refactor] 521-521: Too few public methods (1/2)
(R0903)
[error] 537-537: Undefined variable 'PersistentVolumeChangeEvent'
(E0602)
[error] 546-546: Undefined variable 'PersistentVolumeChangeEvent'
(E0602)
[refactor] 540-540: Too few public methods (1/2)
(R0903)
[error] 555-555: Undefined variable 'PersistentVolumeChangeEvent'
(E0602)
[refactor] 549-549: Too few public methods (1/2)
(R0903)
[refactor] 558-558: Too few public methods (1/2)
(R0903)
[error] 574-574: Undefined variable 'PersistentVolumeClaimChangeEvent'
(E0602)
[error] 583-583: Undefined variable 'PersistentVolumeClaimChangeEvent'
(E0602)
[refactor] 577-577: Too few public methods (1/2)
(R0903)
[error] 592-592: Undefined variable 'PersistentVolumeClaimChangeEvent'
(E0602)
[refactor] 586-586: Too few public methods (1/2)
(R0903)
[refactor] 595-595: Too few public methods (1/2)
(R0903)
[error] 611-611: Undefined variable 'NetworkPolicyChangeEvent'
(E0602)
[error] 620-620: Undefined variable 'NetworkPolicyChangeEvent'
(E0602)
[refactor] 614-614: Too few public methods (1/2)
(R0903)
[error] 629-629: Undefined variable 'NetworkPolicyChangeEvent'
(E0602)
[refactor] 623-623: Too few public methods (1/2)
(R0903)
[refactor] 632-632: Too few public methods (1/2)
(R0903)
[error] 648-648: Undefined variable 'ConfigMapChangeEvent'
(E0602)
[error] 657-657: Undefined variable 'ConfigMapChangeEvent'
(E0602)
[refactor] 651-651: Too few public methods (1/2)
(R0903)
[error] 666-666: Undefined variable 'ConfigMapChangeEvent'
(E0602)
[refactor] 660-660: Too few public methods (1/2)
(R0903)
[refactor] 669-669: Too few public methods (1/2)
(R0903)
[error] 685-685: Undefined variable 'IngressChangeEvent'
(E0602)
[error] 694-694: Undefined variable 'IngressChangeEvent'
(E0602)
[refactor] 688-688: Too few public methods (1/2)
(R0903)
[error] 703-703: Undefined variable 'IngressChangeEvent'
(E0602)
[refactor] 697-697: Too few public methods (1/2)
(R0903)
[refactor] 706-706: Too few public methods (1/2)
(R0903)
[error] 722-722: Undefined variable 'KubernetesAnyChangeEvent'
(E0602)
[error] 731-731: Undefined variable 'KubernetesAnyChangeEvent'
(E0602)
[refactor] 725-725: Too few public methods (1/2)
(R0903)
[error] 740-740: Undefined variable 'KubernetesAnyChangeEvent'
(E0602)
[refactor] 734-734: Too few public methods (1/2)
(R0903)
[refactor] 743-743: Too few public methods (1/2)
(R0903)
scripts/playbook_generator.py
[error] 208-208: Undefined variable 'action_data'
(E0602)
src/robusta/core/triggers/error_event_trigger.py
[refactor] 13-13: Too few public methods (1/2)
(R0903)
[refactor] 68-68: Too few public methods (1/2)
(R0903)
[refactor] 72-72: Too few public methods (1/2)
(R0903)
🔇 Additional comments (22)
pyproject.toml (2)
115-118: LGTM! Streamlit dependencies align with the playbook generator feature.The addition of Streamlit-related dependencies supports the new playbook generator UI mentioned in the AI summary.
24-24: Verify the Python version upgrade impact across the codebase.Upgrading the minimum Python version from 3.8 to 3.10 is a breaking change that may affect users and deployment environments. Ensure this change is intentional and that all dependencies and deployment scripts support Python 3.10+.
Run the following script to check for any Python version-specific code or configurations that might be affected:
#!/bin/bash # Description: Check for Python version references and compatibility issues # Search for Python version references in the codebase echo "=== Python version references ===" rg -i "python.*3\.[0-9]" --type py -A 2 # Check for any version-specific syntax or imports echo "=== Potential compatibility issues ===" rg "sys\.version_info|platform\.python_version" --type py -A 2 # Check Docker files and CI configurations echo "=== Docker and CI configurations ===" fd -e dockerfile -e yml -e yaml | xargs rg -l "python.*3\.[0-9]".python-version (1)
1-1: Python version specification aligns with pyproject.toml but conflicts with StrEnum usage.The file correctly specifies Python 3.10, matching the pyproject.toml requirement. However, this conflicts with the
StrEnumusage insrc/robusta/core/model/k8s_operation_type.py, which requires Python 3.11+.src/robusta/integrations/kubernetes/autogenerated/models.py (1)
5-5: LGTM! Formatting improvement enhances readability.The single-line dictionary declaration is more concise and readable for this simple case.
src/robusta/integrations/kubernetes/autogenerated/events.py (2)
36-36: LGTM! Import formatting improvement enhances readability.Consolidating the import to a single line improves code cleanliness.
52-52: LGTM! Import consolidation improves code organization.Grouping related imports on a single line enhances readability and follows Python import conventions.
src/robusta/integrations/kubernetes/autogenerated/v1/models.py (1)
26-26: Verify the availability ofIngressclass from star imports.Static analysis tools flag
Ingressas potentially undefined. Please verify that theIngressclass is properly exported fromhikaru.model.rel_1_26.v1.#!/bin/bash # Description: Check if Ingress is available in the hikaru package # Expected: Ingress should be importable from hikaru.model.rel_1_26.v1 python3 -c " try: from hikaru.model.rel_1_26.v1 import Ingress print('SUCCESS: Ingress class is available') print(f'Ingress class: {Ingress}') except ImportError as e: print(f'ERROR: Cannot import Ingress - {e}') except Exception as e: print(f'ERROR: Other error - {e}') "src/robusta/core/model/base_params.py (1)
198-198: Documentation improvements look good.The clarification of
graph_duration_minutesdescription and the typo fix in the example make the documentation more helpful and accurate.Also applies to: 202-202
scripts/generate_kubernetes_code.py (1)
442-444: Good practice: Adding code formatting recommendations.Adding the print statements to remind developers to run formatting tools after code generation is a good practice for maintaining code quality.
scripts/main_app.py (1)
2-14: Session state usage is now consistent.Good improvement! The code now consistently uses
ssfor session state access throughout the file, addressing the previous review feedback about consistency.src/robusta/integrations/prometheus/trigger.py (1)
54-58: Well-implemented string enum for alert status.The
AlertStatusenum properly inherits from bothstrandEnum, providing type safety while maintaining backward compatibility with string comparisons.scripts/custom_streamlit_pydantic.py (2)
13-23: Function parameters are justified despite static analysis warnings.The 9 parameters provide necessary customization options for the form. Each parameter serves a specific purpose and has appropriate defaults. The implementation is clean and well-documented.
77-90: Excellent error handling implementation.The validation error handling provides clear, user-friendly error messages with proper formatting and location information. The conditional display based on submit button state prevents premature error messages.
src/robusta/core/playbooks/generation.py (1)
63-76: Well-implemented helper methods for trigger-action mapping.The
get_all_triggersandget_triggers_to_actionsmethods provide clean interfaces for the Streamlit UI to enumerate triggers and map them to actions.src/robusta/integrations/kubernetes/autogenerated/triggers.py (2)
13-47: Clean refactoring to class-level attributes with Literal types.The refactoring from instance initialization to class-level Literal attributes improves code clarity and type safety. This pattern is consistently applied across all trigger classes.
716-749: ```shell
#!/bin/bashDisplay the K8sBaseTrigger class along with its methods to see how
kindis used for filtering eventssed -n '1,300p' src/robusta/integrations/kubernetes/base_triggers.py
</details> <details> <summary>src/robusta/integrations/kubernetes/base_triggers.py (1)</summary> `43-47`: **Documentation improvements enhance schema clarity.** The addition of descriptive `Field` metadata improves the self-documenting nature of the Pydantic model and will help with API documentation generation. </details> <details> <summary>src/robusta/core/triggers/error_event_trigger.py (2)</summary> `65-73`: **Good use of Literal types for operation fields.** The refactoring to use `Literal` types in subclasses provides better type safety and makes the fixed operation values explicit at the type level. --- `15-15`: **Verify the change from list to single operation doesn't break existing configurations.** The `operation` field has been changed from supporting multiple operations to a single optional operation. This could be a breaking change if existing configurations use multiple operations. Run the following script to check for existing usage patterns: ```shell #!/bin/bash # Description: Search for WarningEventTrigger configurations that might use multiple operations # Search for YAML configurations fd -e yaml -e yml | xargs rg -A 5 'WarningEventTrigger|on_warning_event' # Search for Python code instantiating these triggers rg -A 5 'WarningEventTrigger\(' --type pyAlso applies to: 38-39
src/robusta/core/triggers/helm_releases_triggers.py (2)
19-23: Excellent refactoring to use enum for status grouping.The introduction of the
HelmStatusesenum centralizes status definitions and improves maintainability. Using.valueto access the list of statuses is the correct approach.Also applies to: 57-57, 102-102
139-156: Clean subclass implementations with Literal types.The simplified constructors and use of
Literaltypes for thedurationfield make the subclasses more declarative and type-safe.scripts/playbook_generator.py (1)
74-74: Verify image paths are correct.The code references local image files. Ensure these paths are valid in the deployment environment.
Run the following script to verify the image files exist:
#!/bin/bash # Description: Check if the referenced image files exist # Check for the image files for img in "helm-release-failed.png" "deployment-image-change.png" "ingress-image-change.png"; do if fd -p "docs/images/$img" | grep -q .; then echo "✓ Found: docs/images/$img" else echo "✗ Missing: docs/images/$img" fi doneAlso applies to: 85-85, 95-95
| from enum import StrEnum | ||
|
|
||
|
|
||
| class K8sOperationType(Enum): | ||
| class K8sOperationType(StrEnum): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: StrEnum is not available in Python 3.10.
StrEnum was introduced in Python 3.11, but pyproject.toml specifies Python 3.10 as the minimum version. This will cause an ImportError at runtime.
Consider one of these solutions:
Option 1 (Recommended): Use a compatibility approach
-from enum import StrEnum
+try:
+ from enum import StrEnum
+except ImportError:
+ # For Python < 3.11
+ from enum import Enum
+ class StrEnum(str, Enum):
+ pass
-class K8sOperationType(StrEnum):
+class K8sOperationType(StrEnum):Option 2: Revert to regular Enum
-from enum import StrEnum
+from enum import Enum
-class K8sOperationType(StrEnum):
+class K8sOperationType(Enum):Option 3: Update Python requirement to 3.11+ in pyproject.toml
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from enum import StrEnum | |
| class K8sOperationType(Enum): | |
| class K8sOperationType(StrEnum): | |
| try: | |
| from enum import StrEnum | |
| except ImportError: | |
| # For Python < 3.11 | |
| from enum import Enum | |
| class StrEnum(str, Enum): | |
| pass | |
| class K8sOperationType(StrEnum): | |
| ... |
🤖 Prompt for AI Agents
In src/robusta/core/model/k8s_operation_type.py at lines 1 to 4, the use of
StrEnum causes an ImportError because it is only available in Python 3.11+,
while the project requires Python 3.10. To fix this, replace StrEnum with a
compatibility solution such as defining a custom StrEnum class or using a
backport library that provides StrEnum functionality. Alternatively, revert to
using the standard Enum class and manually implement string conversion if
needed, or update the Python version requirement in pyproject.toml to 3.11 or
higher if feasible.
| namespace_prefix=namespace_prefix, | ||
| labels_selector=labels_selector, | ||
| ) | ||
| kind: Literal[\"{resource}\"] = \"{resource}\" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical bug: KubernetesAny triggers incorrectly set kind to last resource.
The KubernetesAnyTrigger classes are incorrectly using {resource} in their kind attribute, which resolves to the last resource from the outer loop ("Ingress"). This should be "Any" for these triggers.
- kind: Literal["{resource}"] = "{resource}"
+ kind: Literal["Any"] = "Any"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| kind: Literal[\"{resource}\"] = \"{resource}\" | |
| kind: Literal["Any"] = "Any" |
🤖 Prompt for AI Agents
In scripts/generate_kubernetes_code.py at line 384, the kind attribute for
KubernetesAnyTrigger classes is incorrectly set to the last resource name due to
using the variable {resource}. Change the kind attribute value to the string
"Any" explicitly for these triggers instead of using {resource} so that it
correctly reflects the intended kind.
| actions = find_playbook_actions(args.directory) | ||
| generator = ExamplesGenerator() | ||
| triggers = generator.get_all_triggers() | ||
| trigger_to_actions = generator.get_triggers_to_actions(actions) | ||
| print(trigger_to_actions) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused variable assignment.
The triggers variable is assigned but never used.
actions = find_playbook_actions(args.directory)
generator = ExamplesGenerator()
- triggers = generator.get_all_triggers()
trigger_to_actions = generator.get_triggers_to_actions(actions)
print(trigger_to_actions)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| actions = find_playbook_actions(args.directory) | |
| generator = ExamplesGenerator() | |
| triggers = generator.get_all_triggers() | |
| trigger_to_actions = generator.get_triggers_to_actions(actions) | |
| print(trigger_to_actions) | |
| actions = find_playbook_actions(args.directory) | |
| generator = ExamplesGenerator() | |
| trigger_to_actions = generator.get_triggers_to_actions(actions) | |
| print(trigger_to_actions) |
🧰 Tools
🪛 Ruff (0.11.9)
77-77: Local variable triggers is assigned to but never used
Remove assignment to unused variable triggers
(F841)
🤖 Prompt for AI Agents
In scripts/generate_playbook_descriptions.py around lines 75 to 79, the variable
'triggers' is assigned the result of 'generator.get_all_triggers()' but is never
used afterwards. Remove the assignment to 'triggers' to clean up the code and
avoid unused variable warnings.
| def find_playbook_actions(scripts_root): | ||
| python_files = glob.glob(f"{scripts_root}/*.py") | ||
| actions = [] | ||
|
|
||
| for script in python_files: | ||
| filename = os.path.basename(script) | ||
| (module_name, ext) = os.path.splitext(filename) | ||
| spec = importlib.util.spec_from_file_location(module_name, script) | ||
| module = importlib.util.module_from_spec(spec) | ||
| spec.loader.exec_module(module) | ||
|
|
||
| playbooks = inspect.getmembers( | ||
| module, | ||
| lambda f: Action.is_action(f), | ||
| ) | ||
| for _, func in playbooks: | ||
| action = Action(func) | ||
| actions.append(action) | ||
|
|
||
| #description = PlaybookDescription( | ||
| # function_name=func.__name__, | ||
| # builtin_trigger_params=func.__playbook["default_trigger_params"], | ||
| # docs=inspect.getdoc(func), | ||
| # src=inspect.getsource(func), | ||
| # src_file=inspect.getsourcefile(func), | ||
| # action_params=get_params_schema(func), | ||
| #) | ||
| #print(description.json(), "\n\n") | ||
|
|
||
| return actions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Remove code duplication - this function already exists in generate_playbook_descriptions.py.
This function is an exact duplicate of the one in scripts/generate_playbook_descriptions.py. To follow DRY principles, keep it in one location and import it where needed.
Since this function is used by scripts, it would be better to keep it in the scripts file and import it here if needed, or move it to a shared utilities module. The duplicate function and its associated imports (lines 14-22) should be removed from this file.
🤖 Prompt for AI Agents
In src/robusta/core/playbooks/generation.py around lines 157 to 186, the
function find_playbook_actions is duplicated from
scripts/generate_playbook_descriptions.py. Remove this function and its related
imports (lines 14-22) from generation.py, then import find_playbook_actions from
generate_playbook_descriptions.py or a shared utilities module to avoid
duplication and follow DRY principles.
| import argparse | ||
| import glob | ||
| import importlib | ||
| import inspect | ||
| import os | ||
| from typing import Callable | ||
|
|
||
| from pydantic import BaseModel | ||
| from robusta.api import Action |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix duplicate and unused imports.
Multiple import issues need to be resolved:
argparseis imported but unusedCallableis already imported on line 4BaseModelis imported but unusedActionis already imported on line 9
-import argparse
-import glob
-import importlib
-import inspect
-import os
-from typing import Callable
-
-from pydantic import BaseModel
-from robusta.api import ActionCommittable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Ruff (0.11.9)
14-14: argparse imported but unused
Remove unused import: argparse
(F401)
19-19: Redefinition of unused Callable from line 4
Remove definition: Callable
(F811)
21-21: pydantic.BaseModel imported but unused
Remove unused import: pydantic.BaseModel
(F401)
22-22: Redefinition of unused Action from line 9
(F811)
🤖 Prompt for AI Agents
In src/robusta/core/playbooks/generation.py around lines 14 to 22, remove the
unused imports argparse and BaseModel, and eliminate the duplicate imports of
Callable and Action by keeping only one import statement for each. This will
clean up the import section and avoid redundancy.
| from typing import List | ||
| from typing import List, Literal, Optional | ||
| from pydantic import Field | ||
| from enum import Enum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused import.
The enum.Enum import is not used in this file.
Apply this diff to remove the unused import:
-from enum import Enum📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| from enum import Enum |
🧰 Tools
🪛 Ruff (0.11.9)
3-3: enum.Enum imported but unused
Remove unused import: enum.Enum
(F401)
🤖 Prompt for AI Agents
In src/robusta/core/triggers/error_event_trigger.py at line 3, remove the unused
import statement "from enum import Enum" since it is not used anywhere in the
file.
| import logging | ||
| import sys |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused imports.
The logging and sys imports are not used in this file.
Apply this diff to remove the unused imports:
-import logging
-import sys📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import logging | |
| import sys |
🧰 Tools
🪛 Ruff (0.11.9)
3-3: logging imported but unused
Remove unused import: logging
(F401)
4-4: sys imported but unused
Remove unused import: sys
(F401)
🤖 Prompt for AI Agents
In src/robusta/core/triggers/helm_releases_triggers.py at lines 3 to 4, remove
the unused imports of logging and sys since they are not used anywhere in the
file. Simply delete these two import lines to clean up the code.
| from typing import List, Set, Optional, Literal | ||
| from enum import Enum |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused imports.
Several imports are not used in this file.
Apply this diff to remove the unused imports:
-from typing import List, Set, Optional, Literal
+from typing import Optional
-from enum import Enum
-import streamlit_pydantic as sp
-from pydantic import BaseModel
-from custom_streamlit_pydantic import modified_pydantic_form, GroupOptionalFieldsStrategy
+from custom_streamlit_pydantic import modified_pydantic_formAlso applies to: 14-14, 17-17, 20-20
🧰 Tools
🪛 Ruff (0.11.9)
8-8: typing.List imported but unused
Remove unused import
(F401)
8-8: typing.Set imported but unused
Remove unused import
(F401)
8-8: typing.Optional imported but unused
Remove unused import
(F401)
8-8: typing.Literal imported but unused
Remove unused import
(F401)
9-9: enum.Enum imported but unused
Remove unused import: enum.Enum
(F401)
🤖 Prompt for AI Agents
In scripts/playbook_generator.py around lines 8-9, 14, 17, and 20, there are
unused imports such as List, Set, Optional, Literal, and Enum that should be
removed to clean up the code. Review the imports on these lines and delete any
that are not referenced elsewhere in the file.
| ) | ||
|
|
||
| trigger_dict = ss["trigger_data"].dict(exclude_defaults=True) | ||
| action_data = ss['action_data'] or {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused variable assignment.
The action_data variable is assigned but never used.
Apply this diff to remove the unused assignment:
-action_data = ss['action_data'] or {}
playbook = {
"customPlaybooks": [
OrderedDict([("triggers", [{ss['trigger_name']: trigger_dict}]), ("actions", [{ss['action_name']: {}}])])
]
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| action_data = ss['action_data'] or {} | |
| playbook = { | |
| "customPlaybooks": [ | |
| OrderedDict([("triggers", [{ss['trigger_name']: trigger_dict}]), ("actions", [{ss['action_name']: {}}])]) | |
| ] | |
| } |
🧰 Tools
🪛 Ruff (0.11.9)
251-251: Local variable action_data is assigned to but never used
Remove assignment to unused variable action_data
(F841)
🤖 Prompt for AI Agents
In scripts/playbook_generator.py at line 251, the variable action_data is
assigned but never used. Remove the entire assignment statement to eliminate the
unused variable.
| ss["action_data"] = action_obj.params_type(**action_data).dict(exclude_defaults=True) | ||
| ss["action_ready"] = True | ||
| except: | ||
| ss["action_ready"] = True | ||
| pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix undefined variable and improve exception handling.
Line 208 references action_data which is not defined, and line 210 uses a bare except clause.
Apply this diff to fix the issues:
try:
- ss["action_data"] = action_obj.params_type(**action_data).dict(exclude_defaults=True)
+ if ss.get("action_data"):
+ ss["action_data"] = action_obj.params_type(**ss["action_data"]).dict(exclude_defaults=True)
ss["action_ready"] = True
-except:
+except Exception as e:
+ st.warning(f"Error processing action data: {str(e)}")
ss["action_ready"] = True
- pass📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ss["action_data"] = action_obj.params_type(**action_data).dict(exclude_defaults=True) | |
| ss["action_ready"] = True | |
| except: | |
| ss["action_ready"] = True | |
| pass | |
| try: | |
| if ss.get("action_data"): | |
| ss["action_data"] = action_obj.params_type(**ss["action_data"]).dict(exclude_defaults=True) | |
| ss["action_ready"] = True | |
| except Exception as e: | |
| st.warning(f"Error processing action data: {str(e)}") | |
| ss["action_ready"] = True |
🧰 Tools
🪛 Ruff (0.11.9)
208-208: Undefined name action_data
(F821)
210-210: Do not use bare except
(E722)
🪛 Pylint (3.3.7)
[error] 208-208: Undefined variable 'action_data'
(E0602)
🤖 Prompt for AI Agents
In scripts/playbook_generator.py around lines 208 to 212, the variable
action_data is used but not defined, and the except clause is bare, which is bad
practice. Define or correctly reference the variable action_data before using it
in the params_type call, and replace the bare except with a specific exception
type to catch. Also, remove the pass statement and handle the exception
properly, for example by logging or setting action_ready accordingly.
WIP. Do not merge or review yet.