Updating URL's for API usage - #900
Conversation
WalkthroughUpdated documentation link strings across core error messages, multiple toolset Python/YAML definitions, and test fixtures — replacing docs.robusta.dev URLs with holmesgpt.dev equivalents and removing two tags from one test. No functional logic or public API signatures changed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
✨ 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
CodeRabbit Configuration File (
|
|
@pavangudiwada is this ready to merge? |
pavangudiwada
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
holmes/plugins/toolsets/grafana/toolset_grafana_tempo.py (1)
279-281: Wrong error message context (mentions "trace by ID" in tags fetch).This path fetches Tempo tags, not a trace by ID. Update the message to avoid confusing users.
- f"Failed to retrieve trace by ID after retries: {e} \n for URL: {url}" + f"Failed to retrieve Tempo tags: {e} \n for URL: {url}"holmes/plugins/toolsets/opensearch/opensearch.py (2)
66-69: Bug: get_client returns the wrong client due to shadowed variable and incorrect predicate.Current code can return the first client even when host matches a different client.
- for client in clients: - found = any(host in client.hosts for client in clients) - if found: - return client + for client in clients: + if host in client.hosts: + return client
193-193: Avoid class-level mutable default for clients.Clients list is shared across instances; can leak state between runs.
-class OpenSearchToolset(Toolset): - model_config = ConfigDict(arbitrary_types_allowed=True) - clients: List[OpenSearchClient] = [] +class OpenSearchToolset(Toolset): + model_config = ConfigDict(arbitrary_types_allowed=True) + clients: List[OpenSearchClient] @@ def __init__(self): super().__init__( @@ ) + self.clients = []holmes/plugins/toolsets/rabbitmq/toolset_rabbitmq.py (1)
41-47: Error message is misleading when multiple clusters are configured.Branch triggers when cluster_id is missing but clusters > 1; message says “No cluster is configured”.
- elif not cluster_id and len(cluster_ids) > 0: - raise ValueError( - f"No cluster is configured. Possible cluster_id values are: {', '.join(cluster_ids)}" - ) + elif not cluster_id and len(cluster_ids) > 1: + raise ValueError( + f"Multiple clusters are configured; 'cluster_id' is required. Possible values: {', '.join(cluster_ids)}" + )
🧹 Nitpick comments (5)
holmes/plugins/toolsets/runbook/runbook_fetcher.py (1)
60-61: Specify file encoding when reading runbooks.Prevents locale-dependent decode issues.
- with open(runbook_path, "r") as file: + with open(runbook_path, "r", encoding="utf-8") as file:holmes/plugins/toolsets/robusta/robusta.py (1)
216-216: Avoid future churn by centralizing docs URLs.Consider a helper/constant to compose toolset docs URLs so you won’t need repo-wide edits next time.
Apply locally:
- docs_url="https://holmesgpt.dev/data-sources/builtin-toolsets/robusta/", + docs_url=docs_url_for("robusta"),Add once (outside this file), e.g. in holmes/plugins/toolsets/consts.py:
DOCS_TOOLSETS_BASE = "https://holmesgpt.dev/data-sources/builtin-toolsets" def docs_url_for(slug: str) -> str: return f"{DOCS_TOOLSETS_BASE}/{slug}/"Then:
from holmes.plugins.toolsets.consts import docs_url_forholmes/plugins/toolsets/coralogix/toolset_coralogix_logs.py (1)
41-41: Use the shared docs URL helper for consistency.Follow the centralization suggested in robusta to reduce duplication.
- docs_url="https://holmesgpt.dev/data-sources/builtin-toolsets/coralogix-logs/", + docs_url=docs_url_for("coralogix-logs"),And import:
from holmes.plugins.toolsets.consts import docs_url_forholmes/plugins/toolsets/internet/notion.py (1)
121-121: Adopt the shared docs URL helper.Keeps all toolsets uniform and easier to change later.
- docs_url="https://holmesgpt.dev/data-sources/builtin-toolsets/notion/", + docs_url=docs_url_for("notion"),And import:
from holmes.plugins.toolsets.consts import docs_url_forholmes/plugins/toolsets/opensearch/opensearch_logs.py (1)
45-45: Prefer centralized docs URL construction.Same helper as suggested above.
- docs_url="https://holmesgpt.dev/data-sources/builtin-toolsets/opensearch-logs/", + docs_url=docs_url_for("opensearch-logs"),And import:
from holmes.plugins.toolsets.consts import docs_url_for
📜 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.
📒 Files selected for processing (27)
holmes/core/supabase_dal.py(2 hunks)holmes/plugins/toolsets/__init__.py(1 hunks)holmes/plugins/toolsets/argocd.yaml(1 hunks)holmes/plugins/toolsets/aws.yaml(2 hunks)holmes/plugins/toolsets/confluence.yaml(1 hunks)holmes/plugins/toolsets/coralogix/toolset_coralogix_logs.py(1 hunks)holmes/plugins/toolsets/docker.yaml(1 hunks)holmes/plugins/toolsets/grafana/toolset_grafana_loki.py(1 hunks)holmes/plugins/toolsets/grafana/toolset_grafana_tempo.py(1 hunks)holmes/plugins/toolsets/helm.yaml(1 hunks)holmes/plugins/toolsets/internet/internet.py(1 hunks)holmes/plugins/toolsets/internet/notion.py(1 hunks)holmes/plugins/toolsets/kafka.py(1 hunks)holmes/plugins/toolsets/kubernetes.yaml(5 hunks)holmes/plugins/toolsets/kubernetes_logs.py(1 hunks)holmes/plugins/toolsets/kubernetes_logs.yaml(1 hunks)holmes/plugins/toolsets/opensearch/opensearch.py(1 hunks)holmes/plugins/toolsets/opensearch/opensearch_logs.py(1 hunks)holmes/plugins/toolsets/opensearch/opensearch_traces.py(1 hunks)holmes/plugins/toolsets/prometheus/prometheus.py(1 hunks)holmes/plugins/toolsets/rabbitmq/toolset_rabbitmq.py(1 hunks)holmes/plugins/toolsets/robusta/robusta.py(1 hunks)holmes/plugins/toolsets/runbook/runbook_fetcher.py(1 hunks)holmes/plugins/toolsets/slab.yaml(1 hunks)tests/llm/fixtures/test_ask_holmes/111_disabled_datadog_traces/test_case.yaml(1 hunks)tests/llm/fixtures/test_ask_holmes/40_disabled_toolset/test_case.yaml(1 hunks)tests/llm/fixtures/test_ask_holmes/41_setup_argo/test_case.yaml(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.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 (mypy is configured in pyproject.toml)
Files:
holmes/plugins/toolsets/kubernetes_logs.pyholmes/plugins/toolsets/opensearch/opensearch_traces.pyholmes/plugins/toolsets/__init__.pyholmes/core/supabase_dal.pyholmes/plugins/toolsets/grafana/toolset_grafana_tempo.pyholmes/plugins/toolsets/opensearch/opensearch.pyholmes/plugins/toolsets/robusta/robusta.pyholmes/plugins/toolsets/coralogix/toolset_coralogix_logs.pyholmes/plugins/toolsets/runbook/runbook_fetcher.pyholmes/plugins/toolsets/kafka.pyholmes/plugins/toolsets/internet/internet.pyholmes/plugins/toolsets/internet/notion.pyholmes/plugins/toolsets/grafana/toolset_grafana_loki.pyholmes/plugins/toolsets/prometheus/prometheus.pyholmes/plugins/toolsets/rabbitmq/toolset_rabbitmq.pyholmes/plugins/toolsets/opensearch/opensearch_logs.py
holmes/plugins/toolsets/**
📄 CodeRabbit inference engine (CLAUDE.md)
Toolsets must live under holmes/plugins/toolsets as either {name}.yaml or a {name}/ directory
Files:
holmes/plugins/toolsets/kubernetes_logs.pyholmes/plugins/toolsets/opensearch/opensearch_traces.pyholmes/plugins/toolsets/slab.yamlholmes/plugins/toolsets/__init__.pyholmes/plugins/toolsets/grafana/toolset_grafana_tempo.pyholmes/plugins/toolsets/opensearch/opensearch.pyholmes/plugins/toolsets/robusta/robusta.pyholmes/plugins/toolsets/coralogix/toolset_coralogix_logs.pyholmes/plugins/toolsets/argocd.yamlholmes/plugins/toolsets/kubernetes_logs.yamlholmes/plugins/toolsets/runbook/runbook_fetcher.pyholmes/plugins/toolsets/kafka.pyholmes/plugins/toolsets/internet/internet.pyholmes/plugins/toolsets/internet/notion.pyholmes/plugins/toolsets/grafana/toolset_grafana_loki.pyholmes/plugins/toolsets/prometheus/prometheus.pyholmes/plugins/toolsets/rabbitmq/toolset_rabbitmq.pyholmes/plugins/toolsets/helm.yamlholmes/plugins/toolsets/confluence.yamlholmes/plugins/toolsets/opensearch/opensearch_logs.pyholmes/plugins/toolsets/docker.yamlholmes/plugins/toolsets/aws.yamlholmes/plugins/toolsets/kubernetes.yaml
tests/llm/**/test_case.yaml
📄 CodeRabbit inference engine (CLAUDE.md)
Eval test cases may declare runbooks in test_case.yaml using either runbooks: {} or runbooks: {catalog: [...]}; if omitted, defaults are used
Files:
tests/llm/fixtures/test_ask_holmes/41_setup_argo/test_case.yamltests/llm/fixtures/test_ask_holmes/111_disabled_datadog_traces/test_case.yamltests/llm/fixtures/test_ask_holmes/40_disabled_toolset/test_case.yaml
tests/llm/**/*.{yaml,yml}
📄 CodeRabbit inference engine (CLAUDE.md)
tests/llm/**/*.{yaml,yml}: Each LLM eval test must use a dedicated Kubernetes namespace named app-
For Kubernetes-related eval assets, always use Secrets for scripts; do not embed scripts in inline manifests or ConfigMaps
Files:
tests/llm/fixtures/test_ask_holmes/41_setup_argo/test_case.yamltests/llm/fixtures/test_ask_holmes/111_disabled_datadog_traces/test_case.yamltests/llm/fixtures/test_ask_holmes/40_disabled_toolset/test_case.yaml
tests/llm/**
📄 CodeRabbit inference engine (CLAUDE.md)
Resource and file naming in evals should be neutral and must not hint at the problem (avoid names like broken-pod or crashloop-app)
Files:
tests/llm/fixtures/test_ask_holmes/41_setup_argo/test_case.yamltests/llm/fixtures/test_ask_holmes/111_disabled_datadog_traces/test_case.yamltests/llm/fixtures/test_ask_holmes/40_disabled_toolset/test_case.yaml
🧠 Learnings (2)
📚 Learning: 2025-05-15T05:13:43.169Z
Learnt from: nherment
PR: robusta-dev/holmesgpt#408
File: holmes/plugins/toolsets/kubernetes_logs.py:90-97
Timestamp: 2025-05-15T05:13:43.169Z
Learning: In the Kubernetes logs toolset for Holmes, both current and previous logs are intentionally fetched and combined for each pod, even though this requires more API calls. This design ensures all logs are captured even when pods restart but retain their name, providing complete diagnostic information.
Applied to files:
holmes/plugins/toolsets/kubernetes_logs.pyholmes/plugins/toolsets/kubernetes_logs.yamlholmes/plugins/toolsets/grafana/toolset_grafana_loki.py
📚 Learning: 2025-08-05T00:42:23.792Z
Learnt from: vishiy
PR: robusta-dev/holmesgpt#782
File: config.example.yaml:31-49
Timestamp: 2025-08-05T00:42:23.792Z
Learning: In robusta-dev/holmesgpt config.example.yaml, the azuremonitorlogs toolset configuration shows "enabled: true" as an example of how to enable the toolset, not as a default setting. The toolset is disabled by default and requires explicit enablement in user configurations.
Applied to files:
holmes/plugins/toolsets/argocd.yamlholmes/plugins/toolsets/kubernetes_logs.yamlholmes/plugins/toolsets/helm.yamlholmes/plugins/toolsets/aws.yamlholmes/plugins/toolsets/kubernetes.yaml
⏰ 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: Pre-commit checks
- GitHub Check: llm_evals
- GitHub Check: Pre-commit checks
🔇 Additional comments (29)
holmes/plugins/toolsets/grafana/toolset_grafana_tempo.py (1)
556-556: Confirm slug: "grafanatempo" vs "grafana-tempo".The path looks inconsistent with other toolset slugs. Please verify the correct docs URL.
Apply if needed:
- docs_url="https://holmesgpt.dev/data-sources/builtin-toolsets/grafanatempo/", + docs_url="https://holmesgpt.dev/data-sources/builtin-toolsets/grafana-tempo/",holmes/plugins/toolsets/slab.yaml (1)
4-4: LGTM — URL updated.Change aligns with the migration to holmesgpt.dev.
holmes/plugins/toolsets/opensearch/opensearch.py (1)
201-201: LGTM — docs_url updated.No functional impact.
holmes/plugins/toolsets/runbook/runbook_fetcher.py (1)
129-129: LGTM — docs_url updated.Matches the docs consolidation.
holmes/plugins/toolsets/rabbitmq/toolset_rabbitmq.py (1)
134-134: LGTM — docs_url updated.Consistent with other toolset URLs.
holmes/plugins/toolsets/prometheus/prometheus.py (1)
858-858: docs_url updated to HolmesGPT — looks correctThe new URL resolves and is the canonical Prometheus toolset page. (holmesgpt.dev)
holmes/plugins/toolsets/grafana/toolset_grafana_loki.py (1)
48-48: Correct Loki docs_url (slug is “grafanaloki”)The updated path exists and points to the Loki toolset page. (holmesgpt.dev)
holmes/plugins/toolsets/helm.yaml (1)
4-4: Helm docs_url migrated correctlyThe new page exists and matches the Helm toolset docs. (holmesgpt.dev)
tests/llm/fixtures/test_ask_holmes/111_disabled_datadog_traces/test_case.yaml (1)
3-3: Test expectation updated to new Datadog docs — OKThe expected link now targets the Datadog toolset page on holmesgpt.dev. (holmesgpt.dev)
holmes/plugins/toolsets/kafka.py (1)
574-574: docs_url is live (HTTP 200) — LGTM.holmes/plugins/toolsets/kubernetes_logs.py (1)
66-66: Kubernetes logs toolset docs_url updated — LGTM.Matches the standardized data-sources path; no functional impact.
holmes/plugins/toolsets/internet/internet.py (1)
271-271: Internet toolset docs_url updated — LGTM.Consistent with the repo-wide migration.
holmes/plugins/toolsets/robusta/robusta.py (1)
216-216: Docs URL updated — LGTM.Change aligns with the PR goal and keeps trailing slash consistency.
holmes/plugins/toolsets/coralogix/toolset_coralogix_logs.py (1)
41-41: Docs URL switched to holmesgpt.dev — looks good.holmes/plugins/toolsets/internet/notion.py (1)
121-121: Docs URL updated correctly.holmes/plugins/toolsets/opensearch/opensearch_logs.py (1)
45-45: Docs URL update — approved.holmes/core/supabase_dal.py (2)
132-147: Updated guidance links — LGTM.Strings now point to relevant holmesgpt.dev pages and read clearly.
132-147: All referenced error-guidance URLs are valid
Headless check confirms each URL returns HTTP 200 OK.holmes/plugins/toolsets/confluence.yaml (1)
4-4: LGTM: docs_url migrated to holmesgpt.devMatches PR scope; no behavioral impact.
holmes/plugins/toolsets/kubernetes_logs.yaml (1)
4-4: LGTM: docs_url updatedChange is scoped to documentation only; tool definitions unchanged.
holmes/plugins/toolsets/docker.yaml (1)
4-4: LGTM: docs_url updatedConsistent with other toolsets’ migration to holmesgpt.dev.
holmes/plugins/toolsets/aws.yaml (2)
4-4: LGTM: AWS security toolset docs_url updatedNo runtime effect; aligns with new docs site.
45-45: LGTM: AWS RDS toolset docs_url updatedConsistent with the security section; no other changes.
holmes/plugins/toolsets/kubernetes.yaml (5)
200-200: LGTM: live-metrics docs_url updatedConsistent with core; no functional changes.
222-222: LGTM: kube-prometheus-stack docs_url updatedDocs link unified to the Kubernetes toolset landing page.
234-234: LGTM: krew-extras docs_url updatedChange is doc-only; prerequisites/tools intact.
250-250: LGTM: kube-lineage-extras docs_url updatedConsistent URL scheme; no other edits.
4-4: Manual docs URL sanity check requiredThe verification script failed to collect
docs_urlentries. Please manually check for:
- any remaining
docs.robusta.devreferences- all
docs_urlvalues inholmes/plugins/toolsets/*.yamlusehttps://…and end with a trailing slash/holmes/plugins/toolsets/argocd.yaml (1)
4-4: docs_url availability confirmed
https://holmesgpt.dev/data-sources/builtin-toolsets/argocd/returns HTTP 200 — LGTM.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/llm/fixtures/test_ask_holmes/55_kafka_runbook/fetch_webpagehttps_docs.robusta.dev_master_configuration_holmesgpt_toolsets_kafka.html.txt (1)
1-2: Rename Kafka fixture file to reflect holmesgpt.dev domain
- Rename
tests/llm/fixtures/test_ask_holmes/55_kafka_runbook/fetch_webpagehttps_docs.robusta.dev_master_configuration_holmesgpt_toolsets_kafka.html.txttofetch_webpagehttps_holmesgpt.dev_data-sources_builtin-toolsets_kafka.html.txt(no references to update)- Prefer asserting on stable page content (e.g., title) rather than exact URL to reduce flakiness
📜 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.
📒 Files selected for processing (2)
tests/llm/fixtures/test_ask_holmes/41_setup_argo/fetch_webpage.txt(1 hunks)tests/llm/fixtures/test_ask_holmes/55_kafka_runbook/fetch_webpagehttps_docs.robusta.dev_master_configuration_holmesgpt_toolsets_kafka.html.txt(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
tests/llm/**
📄 CodeRabbit inference engine (CLAUDE.md)
Resource and file naming in evals should be neutral and must not hint at the problem (avoid names like broken-pod or crashloop-app)
Files:
tests/llm/fixtures/test_ask_holmes/41_setup_argo/fetch_webpage.txttests/llm/fixtures/test_ask_holmes/55_kafka_runbook/fetch_webpagehttps_docs.robusta.dev_master_configuration_holmesgpt_toolsets_kafka.html.txt
🪛 LanguageTool
tests/llm/fixtures/test_ask_holmes/41_setup_argo/fetch_webpage.txt
[grammar] ~1-~1: There might be a mistake here.
Context: .../data-sources/builtin-toolsets/argocd"}} {"schema_version": "robusta:v1.0.0", "st...
(QB_NEW_EN)
tests/llm/fixtures/test_ask_holmes/55_kafka_runbook/fetch_webpagehttps_docs.robusta.dev_master_configuration_holmesgpt_toolsets_kafka.html.txt
[grammar] ~1-~1: There might be a mistake here.
Context: ...v/data-sources/builtin-toolsets/kafka"}} {"schema_version": "robusta:v1.0.0", "st...
(QB_NEW_EN)
[grammar] ~2-~2: There might be a mistake here.
Context: ...v/data-sources/builtin-toolsets/kafka"}} � Kafka - Robusta documentation ...
(QB_NEW_EN)
⏰ 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: Pre-commit checks
- GitHub Check: llm_evals
- GitHub Check: Pre-commit checks
2 participants
No description provided.