aminghadersohi commented on code in PR #33976: URL: https://github.com/apache/superset/pull/33976#discussion_r2176458406
########## superset/mcp_service/api/v1/endpoints.py: ########## @@ -0,0 +1,580 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +"""MCP Service API v1 endpoints""" +import logging +from datetime import datetime, timezone + +from flask import current_app, g, jsonify, request +from marshmallow import ValidationError + +from superset.mcp_service.api import mcp_api +from superset.mcp_service.schemas import ( + MCPDashboardListRequestSchema, MCPDashboardListResponseSchema, MCPDashboardResponseSchema, + MCPDashboardSimpleRequestSchema, MCPErrorResponseSchema, MCPHealthResponseSchema, serialize_mcp_response, + validate_mcp_request, ) + +logger = logging.getLogger(__name__) + +__all__ = [ + "health", + "list_dashboards", + "get_dashboard" +] + + +def requires_api_key(f): + """Decorator to check API key authentication""" + from functools import wraps + + @wraps(f) + def decorated(*args, **kwargs): + logger.debug(f"Authenticating request for endpoint: {f.__name__}") + + # Get API key from config + expected_api_key = current_app.config.get("MCP_API_KEY", "your-secret-api-key-here") + + # Check for API key in Authorization header + auth_header = request.headers.get("Authorization") + if auth_header and auth_header.startswith("Bearer "): + provided_api_key = auth_header[7:] # Remove "Bearer " prefix + else: + # Fallback: check for X-API-Key header + provided_api_key = request.headers.get("X-API-Key") + + if not provided_api_key: + logger.warning(f"Missing API key for endpoint: {f.__name__}") + error_data = { + "error": "Missing Authorization header. Use 'Authorization: Bearer <api-key>' or 'X-API-Key: " + "<api-key>'", + "error_type": "authentication_required", + "timestamp": datetime.now(timezone.utc) + } + serialized_error = serialize_mcp_response(error_data, MCPErrorResponseSchema) + return jsonify(serialized_error), 401 + + if provided_api_key != expected_api_key: + logger.warning(f"Invalid API key for endpoint: {f.__name__}") + error_data = { + "error": "Invalid API key", + "error_type": "authentication_failed", + "timestamp": datetime.now(timezone.utc) + } + serialized_error = serialize_mcp_response(error_data, MCPErrorResponseSchema) + return jsonify(serialized_error), 401 + + logger.debug(f"Authentication successful for endpoint: {f.__name__}") + return f(*args, **kwargs) + + return decorated + + +def serialize_user_object(user): + """Serialize user object to dictionary""" + if not user: + return None + + return { + "id": user.id, + "first_name": user.first_name, + "last_name": user.last_name, + "username": user.username, + "email": getattr(user, 'email', None), + "active": getattr(user, 'active', True), + } + + +def serialize_tag_object(tag): + """Serialize tag object to dictionary""" + if not tag: + return None + + return { + "id": tag.id, + "name": tag.name, + "type": getattr(tag, 'type', None), + } + + +def serialize_role_object(role): + """Serialize role object to dictionary""" + if not role: + return None + + return { + "id": role.id, + "name": role.name, + } + + +def serialize_chart_object(chart): + """Serialize chart object to dictionary""" + if not chart: + return None + + return { + "id": chart.id, + "slice_name": chart.slice_name, + "viz_type": chart.viz_type, + "datasource_name": chart.datasource_name, + "datasource_type": chart.datasource_type, + "url": chart.url, + } + + +@mcp_api.route("/health", methods=["GET"]) +@requires_api_key +def health(): + """Health check endpoint""" + logger.info("Health check requested") + try: + response_data = { + "status": "healthy", + "service": "mcp", + "version": "1.0.0", + "timestamp": datetime.now(timezone.utc) + } + serialized_response = serialize_mcp_response(response_data, MCPHealthResponseSchema) + logger.info("Health check completed successfully") + return jsonify(serialized_response) + except Exception as e: + logger.error(f"Health check failed: {e}") + response_data = { + "status": "unhealthy", + "error": str(e), + "service": "mcp", + "timestamp": datetime.now(timezone.utc) + } + serialized_response = serialize_mcp_response(response_data, MCPHealthResponseSchema) + return jsonify(serialized_response), 503 + + +@mcp_api.route("/list_dashboards", methods=["GET", "POST"]) +@requires_api_key +def list_dashboards(): Review Comment: so right now this double api thing, with rest and fastmcp both having similar functionality and running two separate processes basically even if we call it internally, I am thinking if all the functionality/magic that fab is taking care of can be done/refactored into the DAOs we can essentially get rid of these endpoints and just have it become the "core" module. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
