Skip to content

ROB-1824 fix: use env var for robusta api endpoint for on premise users - #858

Merged
nherment merged 5 commits into
masterfrom
rob-1824_robusta_api_key_use_env_var_for_domain
Aug 19, 2025
Merged

nherment merged 5 commits into
masterfrom
rob-1824_robusta_api_key_use_env_var_for_domain

Conversation

@nherment

Copy link
Copy Markdown
Contributor

No description provided.

@coderabbitai

coderabbitai Bot commented Aug 19, 2025 •

Copy link
Copy Markdown
Contributor

Walkthrough

Added a Config.configure_robusta_ai_model() method and removed the inline Robusta API-key auto-detection from Config._get_llm. holmes/utils/robusta.py now calls the new configure_robusta_ai_model() when the Robusta model is present in config._model_list.

Changes

Cohort / File(s) Summary of Changes
Config: Robusta model setup
holmes/config.py
Added public method configure_robusta_ai_model(self) -> None that injects the Robusta model into self._model_list with base_url = ROBUSTA_API_ENDPOINT and sets the model's api_key from self.api_key when applicable. Removed the inline API-key auto-detection block from _get_llm. Minor whitespace change.
Robusta utils: invoke config hook
holmes/utils/robusta.py
After loading AI credentials and setting config.api_key, now calls config.configure_robusta_ai_model() when ROBUSTA_AI_MODEL_NAME is present in config._model_list. No other logic changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Utils as holmes/utils/robusta.py
  participant Config as Config
  participant _get_llm as Config._get_llm
  participant LLM as DefaultLLM

  note right of Utils #DDEBF7: Load credentials
  Utils->>Config: set config.api_key(...)
  alt ROBUSTA_AI_MODEL_NAME in config._model_list
    Utils->>Config: configure_robusta_ai_model()
    note right of Config #E6F4EA: inject Robusta model + api_key into _model_list
  end

  Config->>_get_llm: build LLM with model_params
  alt model_params.base_url == ROBUSTA_API_ENDPOINT
    _get_llm->>_get_llm: use model_params (no inline api_key fallback)
  else
    _get_llm->>_get_llm: use non-Robusta/default path
  end
  _get_llm->>LLM: Construct DefaultLLM(...)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–30 minutes

Possibly related PRs

Suggested reviewers

  • arikalon1
  • moshemorad

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 Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch rob-1824_robusta_api_key_use_env_var_for_domain

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@nherment
nherment enabled auto-merge (squash) August 19, 2025 06:34

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
holmes/config.py (1)

483-487: Coerce base_url to str to avoid TypeError in the membership check

Since ROBUSTA_API_ENDPOINT is always a string (it defaults to "https://api.robusta.dev" in env_vars.py), you only need to guard against model_params.get("base_url") returning None. Here’s a streamlined diff for lines 483–487 in holmes/config.py:

         if (
-            not api_key
-            and ROBUSTA_API_ENDPOINT in model_params.get("base_url", "")
-            and self.api_key
-        ):
-            api_key = self.api_key.get_secret_value()
+            # Coerce base_url to str to avoid TypeError when it’s None
+        ):
+            pass
+        base_url = str(model_params.get("base_url") or "")
+        if (
+            not api_key
+            and ROBUSTA_API_ENDPOINT in base_url
+            and self.api_key
+        ):
+            api_key = self.api_key.get_secret_value()

Optional backward-compat fallback for hard-coded “robusta.dev” URLs:

-        if (
+        if (
             not api_key
-            and ROBUSTA_API_ENDPOINT in base_url
+            and (
+                ROBUSTA_API_ENDPOINT in base_url
+                or "robusta.dev" in base_url
+            )
             and self.api_key
         ):
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between bcea41f and 97e36d2.

📒 Files selected for processing (1)
  • holmes/config.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

📄 CodeRabbit Inference Engine (CLAUDE.md)

**/*.py: ALWAYS place Python imports at the top of the file, not inside functions or methods
Type hints are required (project is type-checked with mypy)
Use Ruff for formatting and linting (configured in pyproject.toml)

Files:

  • holmes/config.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: nherment
PR: robusta-dev/holmesgpt#610
File: .github/workflows/llm-evaluation.yaml:39-42
Timestamp: 2025-07-08T08:45:41.069Z
Learning: When suggesting improvements to environment variable handling in robusta-dev/holmesgpt, check first if validation logic already exists rather than reimplementing it.
Learnt from: nherment
PR: robusta-dev/holmesgpt#610
File: .github/workflows/llm-evaluation.yaml:39-42
Timestamp: 2025-07-08T08:45:41.069Z
Learning: The robusta-dev/holmesgpt codebase has comprehensive existing validation for Azure environment variables (AZURE_API_BASE, AZURE_API_KEY, AZURE_API_VERSION) and MODEL in tests/llm/utils/classifiers.py, tests/llm/conftest.py, and holmes/core/llm.py. Don't suggest adding redundant validation logic.
📚 Learning: 2025-07-08T08:45:41.069Z
Learnt from: nherment
PR: robusta-dev/holmesgpt#610
File: .github/workflows/llm-evaluation.yaml:39-42
Timestamp: 2025-07-08T08:45:41.069Z
Learning: The robusta-dev/holmesgpt codebase has comprehensive existing validation for Azure environment variables (AZURE_API_BASE, AZURE_API_KEY, AZURE_API_VERSION) and MODEL in tests/llm/utils/classifiers.py, tests/llm/conftest.py, and holmes/core/llm.py. Don't suggest adding redundant validation logic.

Applied to files:

  • holmes/config.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). (3)
  • GitHub Check: llm_evals
  • GitHub Check: Pre-commit checks
  • GitHub Check: Pre-commit checks
🔇 Additional comments (1)
holmes/config.py (1)

483-487: LGTM: Env-based detection improves on-prem compatibility

Switching the Robusta detection to use ROBUSTA_API_ENDPOINT instead of a hard-coded domain is the right move for on-prem flexibility.

moshemorad
moshemorad previously approved these changes Aug 19, 2025

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
holmes/utils/robusta.py (1)

6-10: Call configure_robusta_ai_model() unconditionally and add a return type.

The current guard (if ROBUSTA_AI_MODEL_NAME in config._model_list) prevents configuration in cases where the Robusta model isn’t pre-inserted (e.g., some CLI or config flows). Let configure_robusta_ai_model() determine eligibility (especially after the fix in Config). Also, add the missing return type per project type-hinting guidelines.

Apply this diff:

-def load_robusta_api_key(dal: SupabaseDal, config: Config):
-    if ROBUSTA_AI_MODEL_NAME in config._model_list:
-        account_id, token = dal.get_ai_credentials()
-        config.api_key = SecretStr(f"{account_id} {token}")
-        config.configure_robusta_ai_model()
+def load_robusta_api_key(dal: SupabaseDal, config: Config) -> None:
+    account_id, token = dal.get_ai_credentials()
+    config.api_key = SecretStr(f"{account_id} {token}")
+    config.configure_robusta_ai_model()
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 97e36d2 and 8f7c8af.

📒 Files selected for processing (2)
  • holmes/config.py (1 hunks)
  • holmes/utils/robusta.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

📄 CodeRabbit Inference Engine (CLAUDE.md)

**/*.py: ALWAYS place Python imports at the top of the file, not inside functions or methods
Type hints are required (project is type-checked with mypy)
Use Ruff for formatting and linting (configured in pyproject.toml)

Files:

  • holmes/utils/robusta.py
  • holmes/config.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: nherment
PR: robusta-dev/holmesgpt#610
File: .github/workflows/llm-evaluation.yaml:39-42
Timestamp: 2025-07-08T08:45:41.069Z
Learning: When suggesting improvements to environment variable handling in robusta-dev/holmesgpt, check first if validation logic already exists rather than reimplementing it.
🧬 Code Graph Analysis (1)
holmes/utils/robusta.py (1)
holmes/config.py (1)
  • configure_robusta_ai_model (145-151)
⏰ 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). (3)
  • GitHub Check: llm_evals
  • GitHub Check: Pre-commit checks
  • GitHub Check: Pre-commit checks

Comment thread holmes/config.py
@github-actions

Copy link
Copy Markdown
Contributor

Results of HolmesGPT evals

  • ask_holmes: 23/39 test cases were successful, 0 regressions, 2 skipped, 14 mock failures
Test suite Test case Status
ask 01_how_many_pods ✅
ask 02_what_is_wrong_with_pod ✅
ask 03_what_is_the_command_to_port_forward 🔧
ask 04_related_k8s_events ↪️
ask 05_image_version 🔧
ask 09_crashpod ✅
ask 10_image_pull_backoff 🔧
ask 11_init_containers ✅
ask 14_pending_resources ✅
ask 15_failed_readiness_probe 🔧
ask 17_oom_kill ✅
ask 18_crash_looping_v2 🔧
ask 19_detect_missing_app_details 🔧
ask 20_long_log_file_search 🔧
ask 24_misconfigured_pvc 🔧
ask 28_permissions_error ✅
ask 29_events_from_alert_manager ↪️
ask 39_failed_toolset 🔧
ask 41_setup_argo ✅
ask 42_dns_issues_steps_new_tools 🔧
ask 43_current_datetime_from_prompt ✅
ask 45_fetch_deployment_logs_simple ✅
ask 51_logs_summarize_errors 🔧
ask 53_logs_find_term ✅
ask 54_not_truncated_when_getting_pods ✅
ask 59_label_based_counting ✅
ask 60_count_less_than 🔧
ask 61_exact_match_counting ✅
ask 63_fetch_error_logs_no_errors ✅
ask 79_configmap_mount_issue 🔧
ask 83_secret_not_found 🔧
ask 86_configmap_like_but_secret 🔧
ask 93_calling_datadog ✅
ask 93_calling_datadog ✅
ask 93_calling_datadog ✅
ask 97_logs_clarification_needed 🔧
ask 110_k8s_events_image_pull 🔧
ask 24a_misconfigured_pvc_basic 🔧
ask 13a_pending_node_selector_basic 🔧

Legend

  • ✅ the test was successful
  • ↪️ the test was skipped
  • ⚠️ the test failed but is known to be flaky or known to fail
  • 🔧 the test failed due to mock data issues (not a code regression)
  • ❌ the test failed and should be fixed before merging the PR

@nherment
nherment merged commit 215f9b4 into master Aug 19, 2025
@nherment
nherment deleted the rob-1824_robusta_api_key_use_env_var_for_domain branch August 19, 2025 11:01
@coderabbitai coderabbitai Bot mentioned this pull request Aug 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants