Skip to content

Commit e9fa6ce

Browse files
Rename RabbitMQ config fields for consistency and add backward compatibility (#1495)
## Summary This PR renames RabbitMQ toolset configuration fields to improve naming consistency and clarity, while maintaining full backward compatibility with existing configurations. ## Key Changes - **Field Renames:** - `management_url` → `api_url` (more accurately describes the RabbitMQ Management API endpoint) - `request_timeout_seconds` → `timeout_seconds` (shorter, more concise naming) - **Backward Compatibility:** - Added deprecated field mappings in `RabbitMQClusterConfig._deprecated_mappings` to automatically migrate old field names to new ones - Old configurations using `management_url` and `request_timeout_seconds` will continue to work without modification - Deprecation warnings are logged when old field names are used - **Documentation Updates:** - Updated all documentation examples to use new field names (`api_url` and `timeout_seconds`) - Updated configuration examples in docstrings and test fixtures - **Code Updates:** - Updated all internal references from `config.management_url` to `config.api_url` - Updated all internal references from `config.request_timeout_seconds` to `config.timeout_seconds` - Updated test fixtures and expected outputs to reflect new field names - **Test Coverage:** - Added comprehensive test suite (`TestRabbitMQConfigBackwardCompatibility`) covering: - Deprecated field migration with warning logs - New field names without warnings - Equivalence between old and new configurations - Precedence when both old and new fields are provided ## Implementation Details The backward compatibility is implemented using Pydantic's field validation mechanism through the `ToolsetConfig` base class, which automatically maps deprecated field names to their new equivalents while logging appropriate warnings for visibility. https://claude.ai/code/session_01UaU7bba5JVNMXujWwWr1Ec <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * RabbitMQ toolset now exposes cluster endpoints as api_url and uses timeout_seconds for request timeouts (backward-compatible with old names). * **Documentation** * Docs updated to replace management_url → api_url and request_timeout_seconds → timeout_seconds across examples and guides. * **Tests** * Added tests to verify backward compatibility, deprecation warnings, and precedence between old and new RabbitMQ config fields. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Signed-off-by: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
1 parent 7d99b20 commit e9fa6ce

File tree

7 files changed

+103
-34
lines changed

7 files changed

+103
-34
lines changed

docs/data-sources/builtin-toolsets/rabbitmq.md

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ By enabling this toolset, HolmesGPT will be able to detect RabbitMQ partitions,
44

55
This toolset follows a two-step process to detect partition:
66

7-
1. The nodes and partitioning status is obtained by fetching information from the configured `management_url`.
7+
1. The nodes and partitioning status is obtained by fetching information from the configured `api_url`.
88
2. If some nodes are reported as not-running, the toolset will try to contact these nodes individually and deduce any partitioning state for any node that is actually running.
99

1010
## Configuration
@@ -18,7 +18,7 @@ toolsets:
1818
- id: rabbitmq # must be unique across all configured clusters
1919
username: <user>
2020
password: <password>
21-
management_url: <http://rabbitmq.rabbitmq:15672>
21+
api_url: <http://rabbitmq.rabbitmq:15672>
2222
```
2323

2424
## Advanced Configuration
@@ -33,8 +33,8 @@ rabbitmq/core:
3333
- id: rabbitmq # must be unique across all configured clusters
3434
username: <user>
3535
password: <password>
36-
management_url: <http://rabbitmq.rabbitmq:15672>
37-
request_timeout_seconds: 30 # timeout for HTTP requests
36+
api_url: <http://rabbitmq.rabbitmq:15672>
37+
timeout_seconds: 30 # timeout for HTTP requests
3838
verify_ssl: true # SSL certificate verification (default: true)
3939
```
4040
@@ -49,7 +49,7 @@ toolsets:
4949
config:
5050
clusters:
5151
- id: rabbitmq
52-
management_url: https://rabbitmq.internal:15672
52+
api_url: https://rabbitmq.internal:15672
5353
username: <user>
5454
password: <password>
5555
verify_ssl: false # Disable SSL verification (default: true)

holmes/plugins/toolsets/rabbitmq/api.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ class ClusterConnectionStatus(str, Enum):
2121
class RabbitMQClusterConfig(ToolsetConfig):
2222
_deprecated_mappings: ClassVar[Dict[str, Optional[str]]] = {
2323
"verify_certs": "verify_ssl",
24+
"management_url": "api_url",
25+
"request_timeout_seconds": "timeout_seconds",
2426
}
2527

2628
id: str = Field(
@@ -29,8 +31,8 @@ class RabbitMQClusterConfig(ToolsetConfig):
2931
description="Unique identifier for this cluster",
3032
examples=["rabbitmq", "rabbitmq-prod"],
3133
)
32-
management_url: str = Field(
33-
title="Management URL",
34+
api_url: str = Field(
35+
title="API URL",
3436
description="RabbitMQ Management API URL",
3537
examples=[
3638
"http://<your-rabbitmq-server-or-service>:15672",
@@ -48,7 +50,7 @@ class RabbitMQClusterConfig(ToolsetConfig):
4850
description="Password for authentication",
4951
examples=["holmes_password"],
5052
)
51-
request_timeout_seconds: int = Field(
53+
timeout_seconds: int = Field(
5254
default=30,
5355
title="Request Timeout",
5456
description="Request timeout in seconds",
@@ -155,7 +157,7 @@ def make_request(
155157
auth=get_auth(config),
156158
params=params,
157159
json=data,
158-
timeout=config.request_timeout_seconds,
160+
timeout=config.timeout_seconds,
159161
verify=config.verify_ssl,
160162
)
161163
response.raise_for_status()
@@ -188,7 +190,7 @@ def get_status_from_node(
188190
hostname = parts[1]
189191

190192
# Construct the target node's management URL based on the original config's scheme/port
191-
parsed_original_url = urlparse(config.management_url)
193+
parsed_original_url = urlparse(config.api_url)
192194
scheme = parsed_original_url.scheme or "http"
193195
port = parsed_original_url.port or (
194196
443 if scheme == "https" else 15672
@@ -258,7 +260,7 @@ def get_cluster_status(config: RabbitMQClusterConfig) -> ClusterStatus:
258260
"""
259261
raw_nodes_data: List[Dict] = []
260262
try:
261-
url = get_url(config.management_url, "api/nodes")
263+
url = get_url(config.api_url, "api/nodes")
262264
raw_nodes_data = make_request(
263265
config=config,
264266
method="GET",
@@ -268,7 +270,7 @@ def get_cluster_status(config: RabbitMQClusterConfig) -> ClusterStatus:
268270
config.connection_error = None
269271
except Exception as e:
270272
logging.error(
271-
f"Failed to get primary cluster status from {config.management_url}: {e}"
273+
f"Failed to get primary cluster status from {config.api_url}: {e}"
272274
)
273275
config.connection_status = ClusterConnectionStatus.ERROR
274276
config.connection_error = str(e)

holmes/plugins/toolsets/rabbitmq/toolset_rabbitmq.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,7 @@ class RabbitMQConfig(ToolsetConfig):
3030
clusters: List[RabbitMQClusterConfig] = Field(
3131
title="Clusters",
3232
description="List of RabbitMQ clusters to connect to",
33-
examples=[
34-
[build_config_example(RabbitMQClusterConfig)]
35-
],
33+
examples=[[build_config_example(RabbitMQClusterConfig)]],
3634
)
3735

3836

@@ -78,7 +76,7 @@ def _invoke(self, params: dict, context: ToolInvokeContext) -> StructuredToolRes
7876
available_clusters = [
7977
{
8078
"cluster_id": c.id,
81-
"management_url": c.management_url,
79+
"api_url": c.api_url,
8280
"connection_status": c.connection_status,
8381
}
8482
for c in self.toolset.config.clusters
@@ -169,13 +167,13 @@ def prerequisites_callable(self, config: dict[str, Any]) -> Tuple[bool, str]:
169167
if not env_url:
170168
return (
171169
False,
172-
"RabbitMQ toolset is misconfigured. 'management_url' is required.",
170+
"RabbitMQ toolset is misconfigured. 'api_url' is required.",
173171
)
174172
config = {
175173
"clusters": [
176174
{
177175
"id": "rabbitmq",
178-
"management_url": env_url,
176+
"api_url": env_url,
179177
"username": env_user,
180178
"password": env_pass,
181179
}
@@ -193,7 +191,7 @@ def prerequisites_callable(self, config: dict[str, Any]) -> Tuple[bool, str]:
193191
def _check_clusters_config(self, config: RabbitMQConfig) -> Tuple[bool, str]:
194192
errors = []
195193
for cluster_config in config.clusters:
196-
url = urljoin(cluster_config.management_url, "api/overview")
194+
url = urljoin(cluster_config.api_url, "api/overview")
197195

198196
try:
199197
data = make_request(

tests/llm/fixtures/test_ask_holmes/38_rabbitmq_split_head/list_configured_clusters.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"data": [
88
{
99
"cluster_id": "rabbitmq",
10-
"management_url": "http://localhost:15672",
10+
"api_url": "http://localhost:15672",
1111
"connection_status": "success"
1212
}
1313
],

tests/llm/fixtures/test_ask_holmes/38_rabbitmq_split_head/toolsets.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@ toolsets:
66
- id: rabbitmq
77
username: user
88
password: "{{env.RABBITMQ_PASSWORD}}"
9-
management_url: http://localhost:15672
9+
api_url: http://localhost:15672

tests/llm/fixtures/test_ask_holmes/39_failed_toolset/toolsets.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@ toolsets:
55
- id: rabbitmq
66
username: user
77
password: "incorrect_password"
8-
management_url: http://localhost:15672
8+
api_url: http://localhost:15672
99
kubernetes/core:
1010
enabled: true

tests/utils/test_toolset_config.py

Lines changed: 81 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@
77

88
from holmes.plugins.toolsets.newrelic.newrelic import NewrelicConfig
99
from holmes.plugins.toolsets.prometheus.prometheus import PrometheusConfig
10+
from holmes.plugins.toolsets.rabbitmq.api import RabbitMQClusterConfig
11+
from holmes.plugins.toolsets.servicenow_tables.servicenow_tables import (
12+
ServiceNowTablesConfig,
13+
)
1014
from holmes.utils.pydantic_utils import ToolsetConfig
1115

1216

@@ -119,15 +123,88 @@ def test_new_prometheus_fields_no_warning(self, caplog):
119123
assert "deprecated" not in caplog.text.lower()
120124

121125

126+
class TestRabbitMQConfigBackwardCompatibility:
127+
"""Test backward compatibility for RabbitMQClusterConfig deprecated fields."""
128+
129+
def test_deprecated_rabbitmq_fields(self, caplog):
130+
"""Test that deprecated RabbitMQ config fields are migrated."""
131+
with caplog.at_level(logging.WARNING):
132+
# Use old field names (deprecated)
133+
old_config = RabbitMQClusterConfig(
134+
management_url="http://rabbitmq:15672",
135+
request_timeout_seconds=45,
136+
)
137+
138+
# Verify migration to new field names
139+
assert old_config.api_url == "http://rabbitmq:15672"
140+
assert old_config.timeout_seconds == 45
141+
assert "management_url -> api_url" in caplog.text
142+
assert "request_timeout_seconds -> timeout_seconds" in caplog.text
143+
144+
def test_new_rabbitmq_fields_no_warning(self, caplog):
145+
"""Test that new RabbitMQ field names don't trigger warnings."""
146+
with caplog.at_level(logging.WARNING):
147+
# Use new field names (current)
148+
new_config = RabbitMQClusterConfig(
149+
api_url="http://rabbitmq:15672",
150+
timeout_seconds=30,
151+
)
152+
153+
assert new_config.api_url == "http://rabbitmq:15672"
154+
assert new_config.timeout_seconds == 30
155+
assert "deprecated" not in caplog.text.lower()
156+
157+
def test_old_and_new_configs_produce_same_result(self, caplog):
158+
"""Test that configs created with old and new field names produce identical results."""
159+
with caplog.at_level(logging.WARNING):
160+
# Create config using old field names
161+
old_config = RabbitMQClusterConfig(
162+
id="test-cluster",
163+
management_url="http://rabbitmq:15672",
164+
username="user",
165+
password="pass",
166+
request_timeout_seconds=60,
167+
verify_ssl=False,
168+
)
169+
170+
# Create config using new field names
171+
new_config = RabbitMQClusterConfig(
172+
id="test-cluster",
173+
api_url="http://rabbitmq:15672",
174+
username="user",
175+
password="pass",
176+
timeout_seconds=60,
177+
verify_ssl=False,
178+
)
179+
180+
# Both configs should have identical values
181+
assert old_config.id == new_config.id
182+
assert old_config.api_url == new_config.api_url
183+
assert old_config.username == new_config.username
184+
assert old_config.password == new_config.password
185+
assert old_config.timeout_seconds == new_config.timeout_seconds
186+
assert old_config.verify_ssl == new_config.verify_ssl
187+
188+
def test_new_field_takes_precedence_over_deprecated(self, caplog):
189+
"""Test that new field takes precedence if both old and new are provided."""
190+
with caplog.at_level(logging.WARNING):
191+
config = RabbitMQClusterConfig(
192+
management_url="http://old-url:15672",
193+
api_url="http://new-url:15672",
194+
request_timeout_seconds=30,
195+
timeout_seconds=60,
196+
)
197+
198+
# New fields should take precedence
199+
assert config.api_url == "http://new-url:15672"
200+
assert config.timeout_seconds == 60
201+
202+
122203
class TestServiceNowConfigBackwardCompatibility:
123204
"""Test backward compatibility for ServiceNowTablesConfig deprecated fields."""
124205

125206
def test_deprecated_servicenow_fields(self, caplog):
126207
"""Test that deprecated ServiceNow config fields are migrated."""
127-
from holmes.plugins.toolsets.servicenow_tables.servicenow_tables import (
128-
ServiceNowTablesConfig,
129-
)
130-
131208
with caplog.at_level(logging.WARNING):
132209
old_config = ServiceNowTablesConfig(
133210
api_key="now_test123",
@@ -140,10 +217,6 @@ def test_deprecated_servicenow_fields(self, caplog):
140217

141218
def test_new_servicenow_fields_no_warning(self, caplog):
142219
"""Test that new ServiceNow field names don't trigger warnings."""
143-
from holmes.plugins.toolsets.servicenow_tables.servicenow_tables import (
144-
ServiceNowTablesConfig,
145-
)
146-
147220
with caplog.at_level(logging.WARNING):
148221
new_config = ServiceNowTablesConfig(
149222
api_key="now_test123",
@@ -155,10 +228,6 @@ def test_new_servicenow_fields_no_warning(self, caplog):
155228

156229
def test_deprecated_and_new_servicenow_configs_equivalent(self, caplog):
157230
"""Test that configs created with old and new fields are equivalent."""
158-
from holmes.plugins.toolsets.servicenow_tables.servicenow_tables import (
159-
ServiceNowTablesConfig,
160-
)
161-
162231
# Create config using deprecated field name
163232
with caplog.at_level(logging.WARNING):
164233
old_config = ServiceNowTablesConfig(

0 commit comments

Comments
 (0)