Add deployment context detection for targeted user guidance #2004
Conversation
Adds utility to detect whether user is running via: - CLI (command line) - Helm chart (Robusta) - Helm chart (Holmes standalone) This enables showing deployment-specific guidance for environment variable configuration and other setup instructions. Key features: - DeploymentContext class with method and chart_type detection - get_env_var_guidance() helper for generating targeted help messages - Detection based on PLAYBOOKS_CONFIG_FILE_PATH, INSTALLATION_NAMESPACE, RELEASE_NAME, HOLMES_ENABLED, and HOLMES_STANDALONE env vars - Different guidance for CLI users vs Robusta Helm vs Holmes Helm https://claude.ai/code/session_01DuWGiUa4DB1FBPG5mLmqhD
|
✅ Docker image ready for
Use this tag to pull the image for testing. 📋 Copy commandsgcloud auth configure-docker us-central1-docker.pkg.dev
docker pull us-central1-docker.pkg.dev/robusta-development/temporary-builds/robusta-runner:1a5820c
docker tag us-central1-docker.pkg.dev/robusta-development/temporary-builds/robusta-runner:1a5820c me-west1-docker.pkg.dev/robusta-development/development/robusta-runner-dev:1a5820c
docker push me-west1-docker.pkg.dev/robusta-development/development/robusta-runner-dev:1a5820cPatch Helm values in one line: helm upgrade --install robusta robusta/robusta \
--reuse-values \
--set runner.image=me-west1-docker.pkg.dev/robusta-development/development/robusta-runner-dev:1a5820c |
WalkthroughA new deployment context detection module is introduced that identifies the deployment method (CLI vs Helm) and chart type (Robusta vs Holmes) from environment variables, along with utility functions to generate deployment-specific guidance for environment variable configuration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/test_deployment_context.py`:
- Line 6: Remove the unused import of pytest from the top of the file: delete
the line importing pytest (the lone "import pytest") in
tests/test_deployment_context.py since the file only uses built-in assert
statements and does not reference pytest anywhere else.
🧹 Nitpick comments (2)
src/robusta/core/model/deployment_context.py (1)
27-41: Consider using@dataclassdecorator.The class could benefit from the
@dataclassdecorator to reduce boilerplate and gain automatic__repr__,__eq__, and other dunder methods.♻️ Proposed refactor using dataclass
+from dataclasses import dataclass + + +@dataclass class DeploymentContext: """Detected deployment context information.""" - - def __init__( - self, - method: DeploymentMethod, - chart_type: ChartType, - release_name: Optional[str] = None, - namespace: Optional[str] = None, - ): - self.method = method - self.chart_type = chart_type - self.release_name = release_name - self.namespace = namespace + method: DeploymentMethod + chart_type: ChartType + release_name: Optional[str] = None + namespace: Optional[str] = Nonetests/test_deployment_context.py (1)
88-102: Consider assertingchart_typein these tests.These tests verify that
method == DeploymentMethod.HELMbut don't assert thechart_type. Based on the implementation logic, both cases should result inChartType.ROBUSTA(sinceholmes_enabledis not set).Adding these assertions would make the tests more comprehensive and catch potential regressions in chart type detection.
💚 Proposed enhancement
def test_detects_helm_with_non_default_namespace(self): env = { "INSTALLATION_NAMESPACE": "custom-namespace", } with mock.patch.dict(os.environ, env, clear=True): context = detect_deployment_context() assert context.method == DeploymentMethod.HELM + assert context.chart_type == ChartType.ROBUSTA def test_detects_helm_with_non_default_release_name(self): env = { "RELEASE_NAME": "custom-release", } with mock.patch.dict(os.environ, env, clear=True): context = detect_deployment_context() assert context.method == DeploymentMethod.HELM + assert context.chart_type == ChartType.ROBUSTA
| import os | ||
| from unittest import mock | ||
|
|
||
| import pytest |
There was a problem hiding this comment.
Remove unused pytest import.
The pytest module is imported but not used in this file. The tests use plain assert statements which don't require an explicit import of pytest.
🧹 Proposed fix
-import pytest
-
from robusta.core.model.deployment_context import (📝 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 pytest | |
| from robusta.core.model.deployment_context import ( |
🧰 Tools
🪛 Flake8 (7.3.0)
[error] 6-6: 'pytest' imported but unused
(F401)
🤖 Prompt for AI Agents
In `@tests/test_deployment_context.py` at line 6, Remove the unused import of
pytest from the top of the file: delete the line importing pytest (the lone
"import pytest") in tests/test_deployment_context.py since the file only uses
built-in assert statements and does not reference pytest anywhere else.
Adds utility to detect whether user is running via:
This enables showing deployment-specific guidance for environment
variable configuration and other setup instructions.
Key features:
RELEASE_NAME, HOLMES_ENABLED, and HOLMES_STANDALONE env vars
https://claude.ai/code/session_01DuWGiUa4DB1FBPG5mLmqhD