-
Notifications
You must be signed in to change notification settings - Fork 16
Multi bucket support and index advisor tool addition #20
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: main
Are you sure you want to change the base?
Changes from all commits
b3abd77
77ce027
eda8103
03d0d3c
00a391c
08a3334
859177c
6ce9a88
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,31 +11,60 @@ | |
from mcp.server.fastmcp import Context | ||
|
||
from utils.constants import MCP_SERVER_NAME | ||
from utils.context import ensure_bucket_connection | ||
from utils.context import ensure_cluster_connection, ensure_bucket_connection | ||
|
||
logger = logging.getLogger(f"{MCP_SERVER_NAME}.tools.query") | ||
|
||
|
||
def get_schema_for_collection( | ||
ctx: Context, scope_name: str, collection_name: str | ||
ctx: Context, bucket_name: str, scope_name: str, collection_name: str | ||
) -> list[dict[str, Any]]: | ||
"""Get the schema for a collection in the specified scope. | ||
Returns a dictionary with the schema returned by running INFER on the Couchbase collection. | ||
""" | ||
try: | ||
query = f"INFER {collection_name}" | ||
result = run_sql_plus_plus_query(ctx, scope_name, query) | ||
result = run_sql_plus_plus_query(ctx, bucket_name, scope_name, query) | ||
return result | ||
except Exception as e: | ||
logger.error(f"Error getting schema: {e}") | ||
raise | ||
|
||
def advise_index_for_sql_plus_plus_query( | ||
ctx: Context, bucket_name: str, scope_name: str, query: str | ||
) -> dict[str, Any]: | ||
"""Get an index recommendation from the SQL++ index advisor for a specified query on a specified bucket and scope. | ||
Returns a dictionary with the query advised on, as well as: | ||
1. an array of the current indexes used and their status (or a string indicating no existing indexes available) | ||
2. an array of recommended indexes and/or covering indexes with reasoning (or a string indicating no possible index improvements) | ||
""" | ||
response = {} | ||
|
||
try: | ||
advise_query = f"ADVISE {query}" | ||
result = run_sql_plus_plus_query(ctx, bucket_name, scope_name, advise_query) | ||
|
||
if result and (advice := result[0].get("advice")): | ||
if (advice is not None): | ||
advise_info = advice.get("adviseinfo") | ||
if ( advise_info is not None): | ||
response["current_indexes"] = advise_info.get("current_indexes", "No current indexes") | ||
response["recommended_indexes"] = advise_info.get("recommended_indexes","No index recommendations available") | ||
response["query"]=result[0].get("query","Query statement unavailable") | ||
Comment on lines
+47
to
+53
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The nested I suggest refactoring this to be more concise and Pythonic by flattening the conditional logic and using the walrus operator more effectively. This will improve readability. if result and (advice := result[0].get("advice")):
if advise_info := advice.get("adviseinfo"):
response["current_indexes"] = advise_info.get("current_indexes", "No current indexes")
response["recommended_indexes"] = advise_info.get("recommended_indexes", "No index recommendations available")
response["query"] = result[0].get("query", "Query statement unavailable") |
||
return response | ||
except Exception as e: | ||
logger.error(f"Error running Advise on query: {e}") | ||
raise ValueError(f"Unable to run ADVISE on: {query} for keyspace {bucket_name}.{scope_name}") from e | ||
|
||
def run_sql_plus_plus_query( | ||
ctx: Context, scope_name: str, query: str | ||
ctx: Context, bucket_name: str, scope_name: str, query: str | ||
) -> list[dict[str, Any]]: | ||
"""Run a SQL++ query on a scope and return the results as a list of JSON objects.""" | ||
bucket = ensure_bucket_connection(ctx) | ||
try: | ||
bucket = ensure_bucket_connection(ctx, bucket_name) | ||
except Exception as e: | ||
logger.error(f"Error accessing bucket: {e}") | ||
raise ValueError("Tool does not have access to bucket, or bucket does not exist.") from e | ||
Comment on lines
+63
to
+67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
app_context = ctx.request_context.lifespan_context | ||
read_only_query_mode = app_context.read_only_query_mode | ||
logger.info(f"Running SQL++ queries in read-only mode: {read_only_query_mode}") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,7 +11,7 @@ | |
|
||
from utils.config import get_settings | ||
from utils.constants import MCP_SERVER_NAME | ||
from utils.context import ensure_bucket_connection | ||
from utils.context import ensure_cluster_connection, ensure_bucket_connection | ||
|
||
logger = logging.getLogger(f"{MCP_SERVER_NAME}.tools.server") | ||
|
||
|
@@ -45,38 +45,96 @@ def get_server_configuration_status(ctx: Context) -> dict[str, Any]: | |
} | ||
|
||
|
||
def test_connection(ctx: Context) -> dict[str, Any]: | ||
"""Test the connection to Couchbase cluster and bucket. | ||
def test_connection(ctx: Context, bucket_name: str = None) -> dict[str, Any]: | ||
"""Test the connection to Couchbase cluster and optionally a specified bucket. | ||
Returns connection status and basic cluster information. | ||
""" | ||
cluster_connected = False | ||
bucket_connected = False | ||
try: | ||
bucket = ensure_bucket_connection(ctx) | ||
|
||
# Test basic connectivity by getting bucket name | ||
bucket_name = bucket.name | ||
|
||
return { | ||
"status": "success", | ||
"cluster_connected": True, | ||
"bucket_connected": True, | ||
"bucket_name": bucket_name, | ||
"message": "Successfully connected to Couchbase cluster and bucket", | ||
} | ||
cluster = ensure_cluster_connection(ctx) | ||
cluster_connected = True | ||
if bucket_name is not None: | ||
try: | ||
bucket = ensure_bucket_connection(ctx, bucket_name) | ||
bucket_connected = True | ||
return { | ||
"status": "success", | ||
"cluster_connected": cluster_connected, | ||
"bucket_connected": bucket_connected, | ||
"message": f"Successfully connected to Couchbase cluster and bucket `{bucket_name}`", | ||
} | ||
except Exception as e: | ||
return { | ||
"status": "error", | ||
"cluster_connected": cluster_connected, | ||
"bucket_connected": bucket_connected, | ||
"error": str(e), | ||
"message": f"Failed to connect to bucket named `{bucket_name}`", | ||
} | ||
else: | ||
return { | ||
"status": "success", | ||
"cluster_connected": cluster_connected, | ||
"message": "Successfully connected to Couchbase cluster", | ||
} | ||
except Exception as e: | ||
return { | ||
"status": "error", | ||
"cluster_connected": False, | ||
"bucket_connected": False, | ||
"cluster_connected": cluster_connected, | ||
"error": str(e), | ||
"message": "Failed to connect to Couchbase", | ||
} | ||
|
||
|
||
def get_scopes_and_collections_in_bucket(ctx: Context) -> dict[str, list[str]]: | ||
"""Get the names of all scopes and collections in the bucket. | ||
def get_list_of_buckets_with_settings( | ||
ctx: Context | ||
) -> list[dict[str, Any]]: | ||
"""Get the list of buckets from the Couchbase cluster, including their bucket settings and additional statistics. | ||
Returns a list of comprehensive bucket information objects including settings. | ||
""" | ||
|
||
result = [] | ||
|
||
try: | ||
cluster = ensure_cluster_connection(ctx) | ||
bucket_manager = cluster.buckets() | ||
buckets = bucket_manager.get_all_buckets() | ||
|
||
for bucket_settings in buckets: | ||
# Convert BucketSettings object to dictionary using available attributes | ||
bucket_dict = {"bucket_name": bucket_settings.name} | ||
|
||
# Add basic bucket settings with safe access | ||
for attr in ["bucket_type", "ram_quota", "num_replicas", "replica_indexes", | ||
"flush_enabled", "max_expiry", "compression_mode", | ||
"minimum_durability_level", "storage_backend", "eviction_policy", | ||
"conflict_resolution", "history_retention_collection_default", | ||
"history_retention_bytes", "history_retention_duration"]: | ||
if hasattr(bucket_settings, attr): | ||
value = getattr(bucket_settings, attr) | ||
# If the value has a .value attribute (enum), use that | ||
if hasattr(value, 'value'): | ||
bucket_dict[attr] = value.value | ||
else: | ||
bucket_dict[attr] = value | ||
|
||
result.append(bucket_dict) | ||
|
||
return result | ||
except Exception as e: | ||
logger.error(f"Error getting bucket information: {e}") | ||
raise | ||
|
||
def get_scopes_and_collections_in_bucket(ctx: Context, bucket_name: str) -> dict[str, list[str]]: | ||
"""Get the names of all scopes and collections for a specified bucket. | ||
Returns a dictionary with scope names as keys and lists of collection names as values. | ||
""" | ||
bucket = ensure_bucket_connection(ctx) | ||
try: | ||
bucket = ensure_bucket_connection(ctx, bucket_name) | ||
except Exception as e: | ||
logger.error(f"Error accessing bucket: {e}") | ||
raise ValueError("Tool does not have access to bucket, or bucket does not exist.") from e | ||
Comment on lines
+133
to
+137
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
try: | ||
scopes_collections = {} | ||
collection_manager = bucket.collections() | ||
|
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.
This
try...except
block for handling the bucket connection is duplicated inupsert_document_by_id
anddelete_document_by_id
in this file, as well as inrun_sql_plus_plus_query
(src/tools/query.py
) andget_scopes_and_collections_in_bucket
(src/tools/server.py
).To improve maintainability and adhere to the Don't Repeat Yourself (DRY) principle, I recommend extracting this logic into a common helper function. This would centralize the error handling and make the tool functions cleaner and easier to maintain.