-
Notifications
You must be signed in to change notification settings - Fork 395
[importer] Add API endpoint and serializer for creating tables from files #4164
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: master
Are you sure you want to change the base?
Conversation
|
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.
Pull Request Overview
Adds a new endpoint and serializer to create SQL tables from file data and wires up a placeholder operation.
- Introduces
CreateTableSerializer
to validate request parameters. - Registers the
create_table
API route and implements the view. - Adds a stub
create_table
function in operations for later implementation.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
desktop/core/src/desktop/lib/importer/serializers.py | New CreateTableSerializer with field and interdependent validations |
desktop/core/src/desktop/lib/importer/api.py | Added create_table view using the serializer and operation stub |
desktop/core/src/desktop/lib/importer/operations.py | Stubbed create_table function with signature but no implementation |
desktop/core/src/desktop/api_public_urls_v1.py | Registered the new importer/table/create route |
Comments suppressed due to low confidence (4)
desktop/core/src/desktop/lib/importer/operations.py:739
- Add a detailed docstring for create_table explaining expected parameters, return format, and error conditions.
def create_table(
desktop/core/src/desktop/lib/importer/serializers.py:178
- Add unit tests for CreateTableSerializer to cover defaulting logic for delimiters and validations of external, Kudu, and Iceberg parameters.
class CreateTableSerializer(serializers.Serializer):
desktop/core/src/desktop/lib/importer/api.py:285
- Include integration tests for the create_table endpoint to verify request validation, error responses, and successful payload structure.
@api_view(["POST"])
desktop/core/src/desktop/lib/importer/operations.py:739
- The signature references List, Dict, Any, Optional but those types are not imported; this will cause a NameError at runtime.
def create_table(
@@ -734,3 +734,31 @@ def _map_polars_dtype_to_sql_type(dialect: str, polars_type: str) -> str: | |||
raise ValueError(f"No mapping for Polars dtype {polars_type} in dialect {dialect}") | |||
|
|||
return mapping[polars_type] | |||
|
|||
|
|||
def create_table( |
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.
The function body is empty (pass
), so calls to create_table will silently return None. Implement the table creation logic or raise NotImplementedError until ready.
Copilot uses AI. Check for mistakes.
@@ -173,3 +173,133 @@ def validate(self, data): | |||
raise serializers.ValidationError({"sheet_name": "Sheet name is required for Excel files."}) | |||
|
|||
return data | |||
|
|||
|
|||
class CreateTableSerializer(serializers.Serializer): |
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.
[nitpick] This serializer is very large. Consider extracting file-type–specific and table-format–specific validations into helper methods or nested serializers for readability.
Copilot uses AI. Check for mistakes.
Python Coverage Report •
Pytest Report
|
This pull request introduces a new API endpoint for creating SQL tables from file data, along with the supporting serializer and backend logic. The changes include defining the
create_table
API, implementing the corresponding serializer for request validation, and adding a placeholder function for the table creation operation.Addition of the
create_table
API:desktop/core/src/desktop/api_public_urls_v1.py
: Added a new route^importer/table/create/? for the
create_table` API endpoint.desktop/core/src/desktop/lib/importer/api.py
: Implemented thecreate_table
API, which validates input using theCreateTableSerializer
, processes the table creation request, and returns metadata and the SQL query.Serializer for request validation:
desktop/core/src/desktop/lib/importer/api.py
: Imported theCreateTableSerializer
to validate parameters for thecreate_table
API.desktop/core/src/desktop/lib/importer/serializers.py
: Defined theCreateTableSerializer
class to validate input parameters such as file path, file type, SQL dialect, and table name, and handle interdependent field validation for specific table formats and file types.Backend logic for table creation:
desktop/core/src/desktop/lib/importer/operations.py
: Added a placeholder functioncreate_table
to encapsulate the logic for creating a table based on the provided parameters.