chore(ibis): add the API details for the doc #1142
Conversation
WalkthroughThis pull request refines the API documentation and metadata across the application. A title parameter is now provided to the FastAPI instance. The connection models have been enhanced with descriptive field annotations and examples. Additionally, API routers in versions 2 and 3 have been updated with route descriptions and improved query parameter documentation. A new tool script, Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant S as generate_openapi.py
participant F as FastAPI App
participant FS as FileSystem
U->>S: Run script with version argument
S->>F: Retrieve OpenAPI specification
F-->>S: Return OpenAPI spec
S->>S: Modify spec (remove writeOnly, add tags, set version)
S->>FS: Write modified spec to openapi.yaml
FS-->>S: Confirm file write
S->>U: Output success message
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
ibis-server/tools/generate_openapi.py (1)
1-52: 💡 Verification agent🧩 Analysis chain
Add missing init.py file
The static analysis tool indicates this file is part of an implicit namespace package. Consider adding an
__init__.pyfile to the tools directory to make it a proper package.
🏁 Script executed:
#!/bin/bash # Check if __init__.py exists in the tools directory if [ ! -f "ibis-server/tools/__init__.py" ]; then echo "Missing __init__.py file in the tools directory." fiLength of output: 153
Missing init.py Detected – Please Add It
The shell script confirms that the
ibis-server/tools/__init__.pyfile is missing. An empty__init__.pyfile should be added to theibis-server/toolsdirectory to properly define it as a package.
- Action Item: Create an empty file at
ibis-server/tools/__init__.py.🧰 Tools
🪛 Ruff (0.8.2)
1-1: File
ibis-server/tools/generate_openapi.pyis part of an implicit namespace package. Add an__init__.py.(INP001)
11-11:
(T201)
🧹 Nitpick comments (5)
ibis-server/tools/generate_openapi.py (3)
1-4: Fix typo in commentThere's a typo in the filename on line 3 - "openai.yaml" should be "openapi.yaml" to match the actual output in line 50.
-# It will create a yaml file called `openai.yaml` at the current path. +# It will create a yaml file called `openapi.yaml` at the current path.🧰 Tools
🪛 Ruff (0.8.2)
1-1: File
ibis-server/tools/generate_openapi.pyis part of an implicit namespace package. Add an__init__.py.(INP001)
9-14: Improve error message and fix typoThere are two issues in this section:
- Typo in "arguemnt" (should be "argument")
- The error message doesn't explain what the version number is used for
-# accept a input arguemnt for the version number +# accept an input argument for the version number if len(sys.argv) != 2: - print("Please provide the version number as an argument.") + print("Please provide the version number as an argument. This will be set in the OpenAPI specification.") sys.exit(1)🧰 Tools
🪛 Ruff (0.8.2)
11-11:
(T201)
49-51: Consider making the output path configurableThe output filename is hardcoded. Consider making it configurable via a command-line argument for better flexibility.
You could modify the script to accept an optional output path parameter:
# accept an input argument for the version number -if len(sys.argv) != 2: - print("Please provide the version number as an argument.") +if len(sys.argv) < 2 or len(sys.argv) > 3: + print("Usage: python generate_openapi.py VERSION [OUTPUT_PATH]") + print("VERSION: The version number to set in the OpenAPI specification.") + print("OUTPUT_PATH: Optional. The path where the OpenAPI spec will be saved (default: openapi.yaml)") sys.exit(1) version = sys.argv[1] +output_path = sys.argv[2] if len(sys.argv) == 3 else "openapi.yaml" ... -with open("openapi.yaml", "w") as f: +with open(output_path, "w") as f: yaml.dump(openapi_spec, f, default_flow_style=False)ibis-server/app/routers/v2/connector.py (1)
229-233: Clear description for model substitute endpoint.The description accurately explains what this endpoint does, though a minor grammar improvement could be made.
- description="get the SQL which table name is substituted", + description="get the SQL with table names substituted",ibis-server/app/model/__init__.py (1)
178-190: Good PostgreSQL connection documentation.Each field has clear descriptions and appropriate examples, though there's an inconsistency in the examples format.
The
portfield on line 181 usesexamples=[5432](without quotes) while other numeric ports use string values likeexamples=["8080"]. Consider using a consistent format across all fields.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
ibis-server/app/main.py(1 hunks)ibis-server/app/model/__init__.py(2 hunks)ibis-server/app/routers/v2/connector.py(8 hunks)ibis-server/app/routers/v3/connector.py(6 hunks)ibis-server/tools/README.md(1 hunks)ibis-server/tools/generate_openapi.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
ibis-server/app/routers/v3/connector.py (3)
ibis-server/app/dependencies.py (1)
verify_query_dto(6-7)ibis-server/app/model/data_source.py (1)
DataSource(42-67)ibis-server/app/model/__init__.py (1)
QueryDTO(17-20)
ibis-server/app/routers/v2/connector.py (11)
ibis-server/app/dependencies.py (1)
verify_query_dto(6-7)ibis-server/app/model/data_source.py (1)
DataSource(42-67)ibis-server/app/model/metadata/dto.py (1)
MetadataDTO(9-10)ibis-server/app/model/metadata/factory.py (2)
MetadataFactory(36-42)get_metadata(38-42)ibis-server/app/model/metadata/bigquery.py (1)
get_version(173-174)ibis-server/app/model/metadata/clickhouse.py (1)
get_version(73-74)ibis-server/app/model/metadata/mssql.py (1)
get_version(164-165)ibis-server/app/model/metadata/mysql.py (1)
get_version(120-121)ibis-server/app/model/metadata/postgres.py (1)
get_version(126-127)ibis-server/app/model/metadata/trino.py (1)
get_version(83-84)ibis-server/app/model/metadata/metadata.py (1)
get_version(20-21)
🪛 LanguageTool
ibis-server/tools/README.md
[uncategorized] ~28-~28: Loose punctuation mark.
Context: ...." } ``` - generate_openapi.py: Used to generate the OpenAPI spec. - ...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 Ruff (0.8.2)
ibis-server/tools/generate_openapi.py
1-1: File ibis-server/tools/generate_openapi.py is part of an implicit namespace package. Add an __init__.py.
(INP001)
11-11: print found
(T201)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: ci
🔇 Additional comments (27)
ibis-server/tools/README.md (1)
28-30: Good addition of the new tool documentation!The documentation follows the existing format and provides clear information about the purpose of the
generate_openapi.pytool and its relationship to Redoc and Redocusaurus.🧰 Tools
🪛 LanguageTool
[uncategorized] ~28-~28: Loose punctuation mark.
Context: ...." } ``` -generate_openapi.py: Used to generate the OpenAPI spec. - ...(UNLIKELY_OPENING_PUNCTUATION)
ibis-server/app/main.py (1)
40-40: Good enhancement of API metadata!Adding the title parameter to the FastAPI instance improves the API documentation and provides better context for API users. This change aligns well with the PR objectives of enhancing API documentation.
ibis-server/app/routers/v3/connector.py (4)
28-28: Good addition of tags for API organization!Adding tags to the APIRouter helps with organizing API endpoints in the documentation, making them easier to navigate. This is a good enhancement for the API documentation.
35-39: Good addition of endpoint description!Adding a description to the query endpoint provides context for users about the purpose of this endpoint. This improves the API documentation.
43-50: Excellent parameter documentation improvements!The enhanced descriptions for the
dry_run,cache_enable, andlimitparameters provide better understanding of their purpose and usage. This is particularly helpful for API users to understand the available options.
116-116: Great addition of descriptions to all endpoints!All endpoints now have clear descriptions explaining their purpose, which significantly improves the API documentation. This consistent approach across all endpoints helps users understand the API functionality better.
Also applies to: 136-139, 165-167, 197-200, 215-218
ibis-server/app/routers/v2/connector.py (9)
26-26: Good enhancement with tags addition.Adding tags to the router helps with organizing API endpoints in the OpenAPI documentation, making them more discoverable.
38-43: Excellent documentation addition!Adding a description to the query endpoint improves the OpenAPI documentation and makes the API more self-documenting.
47-55: Well-documented query parameters.These parameter descriptions clearly explain the purpose of each option, which is essential for API consumers. The aliasing is properly maintained for camelCase frontend compatibility while keeping snake_case backend conventions.
116-120: Good documentation for validation endpoint.The description accurately explains the purpose of this endpoint, helping API consumers understand how to use it.
144-149: Clear documentation for metadata tables endpoint.The description provides a concise explanation of what this endpoint does, which aligns well with the underlying implementation.
164-169: Appropriate description for constraints endpoint.The description accurately reflects that this endpoint retrieves constraints from the specified data source.
184-188: Good description for database version endpoint.This description clearly explains that the endpoint returns the version of the specified data source, which matches the implementation that calls the
get_version()method.
193-193: Clear description for dry plan endpoint.The description accurately describes the purpose of this endpoint, which helps API consumers understand what it does.
207-211: Good description for data source-specific dry plan endpoint.The description clearly differentiates this endpoint from the generic dry plan endpoint by specifying it returns dialect-specific SQL.
ibis-server/app/model/__init__.py (12)
76-82: Well-documented BigQuery connection fields.Clear descriptions and practical examples for each field make it easier for API consumers to understand what values to provide.
86-98: Good documentation for Canner connection fields.Each field has a clear description and appropriate examples, which improves the usability of the API.
105-117: Well-documented ClickHouse connection fields.The descriptions and examples are helpful and appropriately describe each field's purpose.
121-139: Good MSSQL connection documentation.Complete documentation with useful examples for all fields, including driver-specific configuration options.
146-167: Well-described MySQL connection fields.Clear descriptions and examples for each field, with good documentation of SSL-related options.
194-206: Well-documented Oracle connection fields.Complete documentation with examples for all Oracle connection parameters.
210-226: Good Snowflake connection documentation.Clear descriptions and examples for all Snowflake-specific connection parameters.
230-247: Well-described Trino connection fields.Comprehensive documentation with examples for all Trino connection parameters.
251-253: Good LocalFile connection documentation.Clear description and example for the URL field.
260-277: Well-documented S3 connection fields.Comprehensive documentation with examples for all S3-specific connection parameters.
281-303: Good MinioFile connection documentation.Clear descriptions and examples for all Minio-specific connection parameters.
307-326:❓ Verification inconclusive
Well-documented GCS connection fields.
Comprehensive documentation with examples for all GCS-specific connection parameters.
The
credentialsfield includes adefault=None, but is being used in a way that suggests it might be required in some cases. Consider documenting when this field is required versus optional.
🏁 Script executed:
#!/bin/bash # Check if GcsFileConnectionInfo is used in any code that requires credentials rg -A 5 "GcsFileConnectionInfo" | grep "credentials"Length of output: 50
Credentials Field Usage: Clarification Needed
The GCS connection fields are well documented overall. However, the
credentialsfield (with itsdefault=None) remains ambiguous. Our initial search for usage ofGcsFileConnectionInfoin contexts that referencecredentialsdid not return any results, so it’s unclear whether this field is truly optional or conditionally required.
- Please manually verify if there are code paths or external usage scenarios where providing
credentialsis mandatory.- If so, update the field’s description to clearly explain when and why
credentialsmust be supplied.- Otherwise, consider confirming its optional nature in the documentation.
goldmedal
force-pushed
the
chore/update-api-doc
branch
from
69fc088 to
04183ec
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
ibis-server/tools/generate_openapi.py (1)
16-30: Add error handling for schema processing.The schema processing logic could potentially encounter errors if the OpenAPI spec structure is unexpected. Consider adding error handling to make the script more robust.
openapi_spec = app.openapi() tags = [] if "components" in openapi_spec and "schemas" in openapi_spec["components"]: - schemas = openapi_spec["components"]["schemas"] - for schema_name, schema in schemas.items(): - if schema_name.endswith("ConnectionInfo") and "properties" in schema: - for prop in schema["properties"].values(): - # To show the model in the doc page. Remove the writeOnly property. - prop.pop("writeOnly", None) - tags.append({ - "name": schema_name, - "x-displayName": schema_name, - "description": f"<SchemaDefinition schemaRef=\"#/components/schemas/{schema_name}\" />", - }) + try: + schemas = openapi_spec["components"]["schemas"] + for schema_name, schema in schemas.items(): + if schema_name.endswith("ConnectionInfo") and "properties" in schema: + for prop in schema["properties"].values(): + # To show the model in the doc page. Remove the writeOnly property. + prop.pop("writeOnly", None) + tags.append({ + "name": schema_name, + "x-displayName": schema_name, + "description": f"<SchemaDefinition schemaRef=\"#/components/schemas/{schema_name}\" />", + }) + except Exception as e: + print(f"Error processing schemas: {str(e)}") + sys.exit(1)
🧹 Nitpick comments (4)
ibis-server/tools/README.md (1)
28-30: Add usage example for the newgenerate_openapi.pytool.The documentation for this new tool follows the pattern of other tool entries but lacks an usage example. For consistency with other tools documentation, consider adding an example command showing how to invoke the script with the version parameter.
- `generate_openapi.py`: Used to generate the OpenAPI spec. - The generated yaml will follow the extension of [redoc](https://redocly.com/docs-legacy/api-reference-docs/spec-extensions). - It's helpful to create the API doc page by [redocusaurus](https://github.com/rohit-gohri/redocusaurus). + - Example + ``` + poetry run python tools/generate_openapi.py 1.0.0 + ```🧰 Tools
🪛 LanguageTool
[uncategorized] ~28-~28: Loose punctuation mark.
Context: ...." } ``` -generate_openapi.py: Used to generate the OpenAPI spec. - ...(UNLIKELY_OPENING_PUNCTUATION)
ibis-server/tools/generate_openapi.py (3)
1-4: Fix typo in comment about output filename.There's a typo in line 3 of the comments where it refers to "openai.yaml" instead of "openapi.yaml" which is the actual filename used in the code.
# This script generates the OpenAPI specification for the application and saves it to a file. # It's used to generate a yaml file that is used for API documentation builded by redoc. -# It will create a yaml file called `openai.yaml` at the current path. +# It will create a yaml file called `openapi.yaml` at the current path.🧰 Tools
🪛 Ruff (0.8.2)
1-1: File
ibis-server/tools/generate_openapi.pyis part of an implicit namespace package. Add an__init__.py.(INP001)
9-12: Fix typo and improve command-line argument handling.There's a typo in the comment ("arguemnt" instead of "argument"). Also, consider using argparse for more robust command-line argument handling.
-# accept a input arguemnt for the version number -if len(sys.argv) != 2: - print("Please provide the version number as an argument.") - sys.exit(1) +# Parse command line arguments +import argparse + +parser = argparse.ArgumentParser(description="Generate OpenAPI specification") +parser.add_argument("version", help="Version number to include in the specification") +args = parser.parse_args() + +version = args.version🧰 Tools
🪛 Ruff (0.8.2)
11-11:
(T201)
49-51: Make output path configurable.The output file path is currently hardcoded. Consider making it configurable through a command-line argument for better flexibility.
-# Parse command line arguments +# Parse command line arguments import argparse parser = argparse.ArgumentParser(description="Generate OpenAPI specification") parser.add_argument("version", help="Version number to include in the specification") +parser.add_argument("--output", "-o", default="openapi.yaml", help="Output file path (default: openapi.yaml)") args = parser.parse_args() version = args.version +output_path = args.output # ... rest of the script ... try: - with open("openapi.yaml", "w") as f: + with open(output_path, "w") as f: yaml.dump(openapi_spec, f, default_flow_style=False) - print(f"Successfully generated OpenAPI specification (version {version}) at openapi.yaml") + print(f"Successfully generated OpenAPI specification (version {version}) at {output_path}") except Exception as e: - print(f"Error writing OpenAPI specification to file: {str(e)}") + print(f"Error writing OpenAPI specification to {output_path}: {str(e)}") sys.exit(1)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
ibis-server/app/main.py(1 hunks)ibis-server/app/model/__init__.py(2 hunks)ibis-server/app/routers/v2/connector.py(8 hunks)ibis-server/app/routers/v3/connector.py(6 hunks)ibis-server/tools/README.md(1 hunks)ibis-server/tools/generate_openapi.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- ibis-server/app/main.py
- ibis-server/app/routers/v3/connector.py
- ibis-server/app/routers/v2/connector.py
- ibis-server/app/model/init.py
🧰 Additional context used
🪛 LanguageTool
ibis-server/tools/README.md
[uncategorized] ~28-~28: Loose punctuation mark.
Context: ...." } ``` - generate_openapi.py: Used to generate the OpenAPI spec. - ...
(UNLIKELY_OPENING_PUNCTUATION)
🪛 Ruff (0.8.2)
ibis-server/tools/generate_openapi.py
1-1: File ibis-server/tools/generate_openapi.py is part of an implicit namespace package. Add an __init__.py.
(INP001)
11-11: print found
(T201)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: ci
Description
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Documentation