bito-code-review[bot] commented on code in PR #37434: URL: https://github.com/apache/superset/pull/37434#discussion_r2729857213
########## docs/scripts/generate-api-index.mjs: ########## @@ -0,0 +1,240 @@ +/** + * 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. + */ + +/** + * Generates a comprehensive API index MDX file from the OpenAPI spec. + * This creates the api.mdx landing page with all endpoints organized by category. + */ + +import fs from 'fs'; +import path from 'path'; +import { fileURLToPath } from 'url'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); + +const SPEC_PATH = path.join(__dirname, '..', 'static', 'resources', 'openapi.json'); +const OUTPUT_PATH = path.join(__dirname, '..', 'docs', 'api.mdx'); Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Incorrect output file path</b></div> <div id="fix"> The output path incorrectly includes an extra 'docs' directory, causing the file to be written to docs/docs/api.mdx instead of docs/api.mdx as referenced in DOCS_CLAUDE.md and the script's comment. </div> <details> <summary> <b>Code suggestion</b> </summary> <blockquote>Check the AI-generated fix before applying</blockquote> <div id="code"> ````suggestion const OUTPUT_PATH = path.join(__dirname, '..', 'api.mdx'); ```` </div> </details> </div> <small><i>Code Review Run #251f5b</i></small> </div> --- Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>) - [ ] Yes, avoid them ########## docs/yarn.lock: ########## @@ -14089,6 +15026,14 @@ whatwg-fetch@^3.6.20: resolved "https://registry.yarnpkg.com/whatwg-fetch/-/whatwg-fetch-3.6.20.tgz#580ce6d791facec91d37c72890995a0b48d31c70"; integrity sha512-EqhiFU6daOA8kpjOWTL0olhVOF3i7OrFzSYiGsEMB8GcXS+RrzauAERX65xMeNWVqxA6HXH2m69Z9LaKKdisfg== +whatwg-url@^5.0.0: + version "5.0.0" + resolved "https://registry.yarnpkg.com/whatwg-url/-/whatwg-url-5.0.0.tgz#966454e8765462e37644d3626f6742ce8b70965d"; + integrity sha512-saE57nupxk6v3HY35+jzBwYa0rKSy0XR8JSxZPwgLr7ys0IBzhGviA1/TUGJLmSVqs8pb9AnvICXEuOHLprYTw== + dependencies: + tr46 "~0.0.3" + webidl-conversions "^3.0.0" + Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Manual yarn.lock edit</b></div> <div id="fix"> yarn.lock is auto-generated and should not be edited manually, as it can lead to inconsistent dependency resolution. These additions appear to be for new transitive dependencies, but package.json lacks the corresponding declarations. </div> </div> <small><i>Code Review Run #251f5b</i></small> </div> --- Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>) - [ ] Yes, avoid them ########## docs/scripts/generate-api-index.mjs: ########## @@ -0,0 +1,240 @@ +/** + * 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. + */ + +/** + * Generates a comprehensive API index MDX file from the OpenAPI spec. + * This creates the api.mdx landing page with all endpoints organized by category. + */ + +import fs from 'fs'; +import path from 'path'; +import { fileURLToPath } from 'url'; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); + +const SPEC_PATH = path.join(__dirname, '..', 'static', 'resources', 'openapi.json'); +const OUTPUT_PATH = path.join(__dirname, '..', 'docs', 'api.mdx'); + +// Category groupings for better organization +const CATEGORY_GROUPS = { + 'Authentication': ['Security'], + 'Core Resources': ['Dashboards', 'Charts', 'Datasets', 'Database'], + 'Data Exploration': ['Explore', 'SQL Lab', 'Queries', 'Datasources', 'Advanced Data Type'], + 'Organization & Customization': ['Tags', 'Annotation Layers', 'CSS Templates'], + 'Sharing & Embedding': [ + 'Dashboard Permanent Link', 'Explore Permanent Link', 'SQL Lab Permanent Link', + 'Embedded Dashboard', 'Dashboard Filter State', 'Explore Form Data' + ], + 'Scheduling & Alerts': ['Report Schedules'], + 'Security & Access Control': [ + 'Security Roles', 'Security Users', 'Security Permissions', + 'Security Resources (View Menus)', 'Security Permissions on Resources (View Menus)', + 'Row Level Security' + ], + 'Import/Export & Administration': ['Import/export', 'CacheRestApi', 'LogRestApi'], + 'User & System': ['Current User', 'User', 'Menu', 'Available Domains', 'AsyncEventsRestApi', 'OpenApi'], +}; + +function slugify(text) { + return text + .toLowerCase() + .replace(/[^a-z0-9]+/g, '-') + .replace(/(^-|-$)/g, ''); +} + +function generateEndpointLink(summary) { + // Convert summary to the slug format used by docusaurus-openapi-docs + return slugify(summary); +} + +function main() { Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Missing error handling</b></div> <div id="fix"> Script lacks error handling for file read/parse failures, which could cause build to fail unexpectedly if OpenAPI spec is invalid. </div> </div> <small><i>Code Review Run #251f5b</i></small> </div> --- Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>) - [ ] Yes, avoid them ########## docs/scripts/fix-openapi-spec.py: ########## @@ -0,0 +1,828 @@ +# 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. + +""" +Fix missing schema references in the OpenAPI spec. + +This script patches the openapi.json file to add any missing schemas +that are referenced but not defined. +""" + +import json # noqa: TID251 - standalone docs script +import sys +from pathlib import Path +from typing import Any + + +def add_missing_schemas(spec: dict[str, Any]) -> tuple[dict[str, Any], list[str]]: + """Add missing schema definitions to the OpenAPI spec.""" + schemas = spec.get("components", {}).get("schemas", {}) + fixed = [] + + # DashboardScreenshotPostSchema - based on superset/dashboards/schemas.py + if "DashboardScreenshotPostSchema" not in schemas: + schemas["DashboardScreenshotPostSchema"] = { + "type": "object", + "properties": { + "dataMask": { + "type": "object", + "description": "An object representing the data mask.", + "additionalProperties": True, + }, + "activeTabs": { + "type": "array", + "items": {"type": "string"}, + "description": "A list representing active tabs.", + }, + "anchor": { + "type": "string", + "description": "A string representing the anchor.", + }, + "urlParams": { + "type": "array", + "items": { + "type": "array", + "items": {"type": "string"}, + "minItems": 2, + "maxItems": 2, + }, + "description": "A list of tuples, each containing two strings.", + }, + }, + } + fixed.append("DashboardScreenshotPostSchema") + + # DashboardNativeFiltersConfigUpdateSchema - based on superset/dashboards/schemas.py + if "DashboardNativeFiltersConfigUpdateSchema" not in schemas: + schemas["DashboardNativeFiltersConfigUpdateSchema"] = { + "type": "object", + "properties": { + "deleted": { + "type": "array", + "items": {"type": "string"}, + "description": "List of deleted filter IDs.", + }, + "modified": { + "type": "array", + "items": {"type": "object"}, + "description": "List of modified filter configurations.", + }, + "reordered": { + "type": "array", + "items": {"type": "string"}, + "description": "List of filter IDs in new order.", + }, + }, + } + fixed.append("DashboardNativeFiltersConfigUpdateSchema") + + # DashboardColorsConfigUpdateSchema - based on superset/dashboards/schemas.py + if "DashboardColorsConfigUpdateSchema" not in schemas: + schemas["DashboardColorsConfigUpdateSchema"] = { + "type": "object", + "properties": { + "color_namespace": { + "type": "string", + "nullable": True, + "description": "The color namespace.", + }, + "color_scheme": { + "type": "string", + "nullable": True, + "description": "The color scheme name.", + }, + "map_label_colors": { + "type": "object", + "additionalProperties": {"type": "string"}, + "description": "Mapping of labels to colors.", + }, + "shared_label_colors": { + "type": "object", + "additionalProperties": {"type": "string"}, + "description": "Shared label colors across charts.", + }, + "label_colors": { + "type": "object", + "additionalProperties": {"type": "string"}, + "description": "Label to color mapping.", + }, + "color_scheme_domain": { + "type": "array", + "items": {"type": "string"}, + "description": "Color scheme domain values.", + }, + }, + } + fixed.append("DashboardColorsConfigUpdateSchema") + + # FormatQueryPayloadSchema - based on superset/sqllab/schemas.py + if "FormatQueryPayloadSchema" not in schemas: + schemas["FormatQueryPayloadSchema"] = { + "type": "object", + "required": ["sql"], + "properties": { + "sql": { + "type": "string", + "description": "The SQL query to format.", + }, + "engine": { + "type": "string", + "nullable": True, + "description": "The database engine.", + }, + "database_id": { + "type": "integer", + "nullable": True, + "description": "The database id.", + }, + "template_params": { + "type": "string", + "nullable": True, + "description": "The SQL query template params as JSON string.", + }, + }, + } + fixed.append("FormatQueryPayloadSchema") + + # get_slack_channels_schema - based on superset/reports/schemas.py + if "get_slack_channels_schema" not in schemas: + schemas["get_slack_channels_schema"] = { + "type": "object", + "properties": { + "search_string": { + "type": "string", + "description": "String to search for in channel names.", + }, + "types": { + "type": "array", + "items": { + "type": "string", + "enum": ["public_channel", "private_channel"], + }, + "description": "Types of channels to search.", + }, + "exact_match": { + "type": "boolean", + "description": "Whether to match channel names exactly.", + }, + }, + } + fixed.append("get_slack_channels_schema") + + if "components" not in spec: + spec["components"] = {} + spec["components"]["schemas"] = schemas + + return spec, fixed + + +def path_to_operation_id(path: str, method: str) -> str: + """Convert a path and method to an operationId.""" + # Remove /api/v1/ prefix + clean_path = path.replace("/api/v1/", "").strip("/") + + # Replace path parameters + clean_path = clean_path.replace("{", "by_").replace("}", "") + + # Create operation name + method_prefix = { + "get": "get", + "post": "create", + "put": "update", + "delete": "delete", + "patch": "patch", + }.get(method.lower(), method.lower()) + + return f"{method_prefix}_{clean_path}".replace("/", "_").replace("-", "_") + + +def path_to_summary(path: str, method: str) -> str: + """Generate a human-readable summary from path and method.""" + # Remove /api/v1/ prefix + clean_path = path.replace("/api/v1/", "").strip("/") + + # Handle path parameters + parts = [] + for part in clean_path.split("/"): + if part.startswith("{") and part.endswith("}"): + param = part[1:-1] + parts.append(f"by {param}") + else: + parts.append(part.replace("_", " ").replace("-", " ")) + + resource = " ".join(parts) + + method_verb = { + "get": "Get", + "post": "Create", + "put": "Update", + "delete": "Delete", + "patch": "Update", + }.get(method.lower(), method.capitalize()) + + return f"{method_verb} {resource}" + + +def add_missing_operation_ids(spec: dict[str, Any]) -> int: + """Add operationId and summary to operations that are missing them.""" + fixed_count = 0 + + for path, methods in spec.get("paths", {}).items(): + for method, details in methods.items(): + if method not in ["get", "post", "put", "delete", "patch"]: + continue + + if not isinstance(details, dict): + continue + + summary = details.get("summary") + operation_id = details.get("operationId") + + if not summary and not operation_id: + details["operationId"] = path_to_operation_id(path, method) + details["summary"] = path_to_summary(path, method) + fixed_count += 1 + + return fixed_count + + +TAG_DESCRIPTIONS = { + "Advanced Data Type": "Advanced data type operations and conversions.", + "Annotation Layers": "Manage annotation layers and annotations for charts.", + "AsyncEventsRestApi": "Real-time event streaming via Server-Sent Events (SSE).", + "Available Domains": "Get available domains for the Superset instance.", + "CSS Templates": "Manage CSS templates for custom dashboard styling.", + "CacheRestApi": "Cache management and invalidation operations.", + "Charts": "Create, read, update, and delete charts (slices).", + "Current User": "Get information about the authenticated user.", + "Dashboard Filter State": "Manage temporary filter state for dashboards.", + "Dashboard Permanent Link": "Permanent links to dashboard states.", + "Dashboards": "Create, read, update, and delete dashboards.", + "Database": "Manage database connections and metadata.", + "Datasets": "Manage datasets (tables) used for building charts.", + "Datasources": "Query datasource metadata and column values.", + "Embedded Dashboard": "Configure embedded dashboard settings.", + "Explore": "Chart exploration and data querying endpoints.", + "Explore Form Data": "Manage temporary form data for chart exploration.", + "Explore Permanent Link": "Permanent links to chart explore states.", + "Import/export": "Import and export Superset assets.", + "LogRestApi": "Access audit logs and activity history.", + "Menu": "Get the Superset menu structure.", + "OpenApi": "Access the OpenAPI specification.", + "Queries": "View and manage SQL Lab query history.", + "Report Schedules": "Configure scheduled reports and alerts.", + "Row Level Security": "Manage row-level security rules for data access.", + "SQL Lab": "Execute SQL queries and manage SQL Lab sessions.", + "SQL Lab Permanent Link": "Permanent links to SQL Lab states.", + "Security": "Authentication and token management.", + "Security Permissions": "View available permissions.", + "Security Permissions on Resources (View Menus)": "Permission-resource mappings.", + "Security Resources (View Menus)": "Manage security resources (view menus).", + "Security Roles": "Manage security roles and their permissions.", + "Security Users": "Manage user accounts.", + "Tags": "Organize assets with tags.", + "User": "User profile and preferences.", +} + + +def generate_code_sample( + method: str, path: str, has_body: bool = False +) -> list[dict[str, str]]: + """Generate code samples for an endpoint in multiple languages.""" + # Clean up path for display + example_path = path.replace("{pk}", "1").replace("{id_or_slug}", "1") + + samples = [] + + # cURL sample + curl_cmd = f'curl -X {method.upper()} "http://localhost:8088{example_path}";' + curl_cmd += ' \\\n -H "Authorization: Bearer $ACCESS_TOKEN"' + if has_body: + curl_cmd += ' \\\n -H "Content-Type: application/json"' + curl_cmd += ' \\\n -d \'{"key": "value"}\'' + + samples.append( + { + "lang": "cURL", + "label": "cURL", + "source": curl_cmd, + } + ) + + # Python sample + if method.lower() == "get": + python_code = f"""import requests + +response = requests.get( + "http://localhost:8088{example_path}";, + headers={{"Authorization": "Bearer " + access_token}} +) +print(response.json())""" + elif method.lower() == "post": + python_code = f"""import requests + +response = requests.post( + "http://localhost:8088{example_path}";, + headers={{"Authorization": "Bearer " + access_token}}, + json={{"key": "value"}} +) +print(response.json())""" + elif method.lower() == "put": + python_code = f"""import requests + +response = requests.put( + "http://localhost:8088{example_path}";, + headers={{"Authorization": "Bearer " + access_token}}, + json={{"key": "value"}} +) +print(response.json())""" + elif method.lower() == "delete": + python_code = f"""import requests + +response = requests.delete( + "http://localhost:8088{example_path}";, + headers={{"Authorization": "Bearer " + access_token}} +) +print(response.status_code)""" + else: + python_code = f"""import requests + +response = requests.{method.lower()}( + "http://localhost:8088{example_path}";, + headers={{"Authorization": "Bearer " + access_token}} +) +print(response.json())""" + + samples.append( + { + "lang": "Python", + "label": "Python", + "source": python_code, + } + ) + + # JavaScript sample + if method.lower() == "get": + js_code = f"""const response = await fetch( + "http://localhost:8088{example_path}";, + {{ + headers: {{ + "Authorization": `Bearer ${{accessToken}}` + }} + }} +); +const data = await response.json(); +console.log(data);""" + elif method.lower() in ["post", "put", "patch"]: + js_code = f"""const response = await fetch( + "http://localhost:8088{example_path}";, + {{ + method: "{method.upper()}", + headers: {{ + "Authorization": `Bearer ${{accessToken}}`, + "Content-Type": "application/json" + }}, + body: JSON.stringify({{ key: "value" }}) + }} +); +const data = await response.json(); +console.log(data);""" + else: + js_code = f"""const response = await fetch( + "http://localhost:8088{example_path}";, + {{ + method: "{method.upper()}", + headers: {{ + "Authorization": `Bearer ${{accessToken}}` + }} + }} +); +console.log(response.status);""" + + samples.append( + { + "lang": "JavaScript", + "label": "JavaScript", + "source": js_code, + } + ) + + return samples Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Undefined variables in code samples</b></div> <div id="fix"> The generated code samples reference undefined variables, causing runtime errors when users copy the examples. </div> </div> <small><i>Code Review Run #251f5b</i></small> </div> --- Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>) - [ ] Yes, avoid them ########## docs/yarn.lock: ########## @@ -8250,16 +8669,24 @@ [email protected]: resolved "https://registry.yarnpkg.com/infima/-/infima-0.2.0-alpha.45.tgz#542aab5a249274d81679631b492973dd2c1e7466"; integrity sha512-uyH0zfr1erU1OohLk0fT4Rrb94AOhguWNOcD9uGrSpRvNB+6gZXUoJX5J0NtvzBO10YZ9PgvA4NFgt+fYg8ojw== [email protected]: - version "2.0.3" - resolved "https://registry.yarnpkg.com/inherits/-/inherits-2.0.3.tgz#633c2c83e3da42a502f52466022480f4208261de"; - integrity sha512-x00IRNXNy63jwGkJmzPigoySHbaqpNuzKbBOmzK+g2OdZpQ9w+sxCN+VSB3ja7IAge2OP2qpfxTjeNcyjmW1uw== +inflight@^1.0.4: + version "1.0.6" + resolved "https://registry.yarnpkg.com/inflight/-/inflight-1.0.6.tgz#49bd6331d7d02d0c09bc910a1075ba8165b56df9"; + integrity sha512-k92I/b08q4wvFscXCLvqfsHCrjrF7yiXsQuIVvVE7N82W3+aqpzuUdBbfhWcy/FZR3/4IgflMgKLOsvPDrGCJA== + dependencies: + once "^1.3.0" + wrappy "1" Review Comment: <div> <div id="suggestion"> <div id="issue"><b>Security: Memory Leak in Dependency</b></div> <div id="fix"> The addition of [email protected] introduces a known memory leak vulnerability that can lead to denial-of-service via unbounded concurrent callbacks, potentially crashing the Node.js process during docs builds or serves. While exploitation requires many concurrent operations, this affects reliability in high-usage scenarios. Consider updating dependencies to avoid this deprecated package. </div> </div> <small><i>Code Review Run #251f5b</i></small> </div> --- Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>) - [ ] Yes, avoid them -- 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]
