feat(tables): TablesAPI.upload for /v1/tables/upload/#40
Conversation
…point Adds a `client.tables.upload(table_name, file, with_headers=True)` helper that wraps the new backend public endpoint. Accepts str | Path | bytes | BinaryIO | FileUpload for the file argument, defaults to the configured organization, and returns a typed TableUploadResponse. Regenerated openapi-python-client artifacts under _generated/ for the new operation + request/response schemas. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Greptile SummaryThis PR adds
Confidence Score: 4/5Safe to merge after addressing the previously flagged to_multipart() serialisation bugs; the TablesAPI wrapper itself guards against both issues at the call site. The high-level TablesAPI.upload() path is correct: it always passes a real UUID (never None) to TableUploadRequest, so the b"None" multipart bug flagged in the previous thread can only be triggered by callers who construct TableUploadRequest directly. The generated files still carry duplicate/unused import noise and the inconsistent encoding in the UUID branch. The resp.parsed result is not guarded against unexpected 2xx responses, which would silently return None to the caller. The three generated files (upload_table.py, table_upload_request.py, table_upload_response.py) still need the cleanup from the prior review threads; table_upload_request.to_multipart() in particular has the None-as-UUID serialisation bug for direct consumers of the generated model. Important Files Changed
Sequence DiagramsequenceDiagram
participant Caller
participant TablesAPI
participant _as_generated_file
participant TableUploadRequest
participant upload_table
participant Backend
Caller->>TablesAPI: upload(table_name, file, ...)
TablesAPI->>TablesAPI: resolve organization_id → UUID or UNSET
TablesAPI->>_as_generated_file: coerce file type
_as_generated_file-->>TablesAPI: File(payload, ...), close_after
TablesAPI->>TableUploadRequest: build(table_name, file, with_headers, org_id)
TableUploadRequest->>TableUploadRequest: to_multipart() → RequestFiles
TablesAPI->>upload_table: sync_detailed(client, body)
upload_table->>Backend: POST /v1/tables/upload/ (multipart/form-data)
Backend-->>upload_table: 201 TableUploadResponse / 400 ErrorResponse
upload_table-->>TablesAPI: "Response[ErrorResponse | TableUploadResponse]"
TablesAPI->>TablesAPI: translate_response (raise on non-2xx)
TablesAPI->>TablesAPI: close payload if close_after
TablesAPI-->>Caller: TableUploadResponse
Reviews (2): Last reviewed commit: "revert(generated): undo manual generated..." | Re-trigger Greptile |
Greptile P1 on #40: `to_multipart()` serialized an explicit None to the literal bytes `b"None"`, which the backend rejects as an invalid UUID. The hand-written `TablesAPI.upload` wrapper never passes None at runtime (it coerces to the configured org id), but anyone calling the generated layer directly would hit it. Manual patch to the generated file — kept narrow so the next openapi-python-client regen produces a clean diff. The duplicate / unused imports Greptile also flagged in the same generated files (P2) are left alone for the same reason; they'll be cleaned up when codegen is re-run upstream. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Greptile P1 on #40: TableUploadRequest's multipart serializer turns an explicit `organization_id=None` into the literal bytes `b"None"`, which the backend rejects as an invalid UUID. The first attempt patched the generated multipart serializer directly, but `check-codegen-drift` (correctly) blocks divergence between checked-in generated code and what openapi-python-client produces. Move the fix to the hand-written wrapper instead: coerce to UUID when a value is available, otherwise pass UNSET so the generated form-field serializer skips the field cleanly. Re-validated against the live dev stack with the canonical CSV. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…apper The earlier `fix(generated)` commit patched openapi-python-client output to handle organization_id=None. That diverged from what the codegen would produce and tripped `check-codegen-drift`. The wrapper now passes UNSET instead of None (previous commit), so the generated layer no longer needs to special-case None — restore it to the canonical openapi-python-client output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@greptile review |
|
Closing per request. |
Summary
client.tables.upload(table_name, file, with_headers=True, organization_id=None)API.fileacceptsstr | Path | bytes | BinaryIO | FileUpload; mime-guesses totext/csvfor.csvnames._generated/api/tables/upload_table.py+TableUploadRequest/TableUploadResponsemodels from the backend openapi.RoeClient.tablesproperty + module__init__exports.Backend PR: https://github.com/roe-ai/roe-main/pull/3245
Test Plan
tests/unit/test_tables.py— 2 unit tests passing (bytes path multipart shape + RoeClient property wiring)./Users/jadenfix/secondary_agent_results_v2.csvreturns 201 with 7 written rows; confirmed in ClickHouse.uv run pytest tests/).🤖 Generated with Claude Code