[python] Improve catalog API type annotations - #10025
Conversation
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The reviewed annotations are consistent with existing implementations and preserve runtime behavior.
Review effort: Lite
Findings: None
What changed in this PR
Improves static type information across the Python catalog API and implementations.
Changes:
- Adds concrete catalog, table, snapshot, partition, tag, and authentication annotations.
- Aligns filesystem, JDBC, and REST catalog signatures.
- Makes REST database properties consistently optional.
| File | Description |
|---|---|
catalog/catalog.py |
Defines typed catalog API contracts. |
catalog/filesystem_catalog.py |
Aligns filesystem catalog annotations. |
catalog/jdbc_catalog.py |
Aligns JDBC catalog annotations. |
catalog/rest/rest_catalog.py |
Adds REST catalog annotations and imports. |
api/rest_api.py |
Accepts optional database properties. |
api/api_request.py |
Types database request options as optional. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
JingsongLi
left a comment
There was a problem hiding this comment.
Requirement fit: SUPPORTED (triage: GO)\nImplementation: CLEAN\n\nThe static contract has end-to-end developer value: Catalog.get_table and the downstream table, builder, snapshot, partition, and tag APIs now remain typed across the supported catalog implementations. I checked the moved runtime imports for cycles and the optional database-property serialization path; I found no actionable P0/P1/P2 issue in the current diff.
Purpose
Fixes #10024.
Callers lose static type information at
Catalog.get_table(): Pyright reportsUnknownfor the returned table and subsequent read-builder calls becauseCatalogdoes not import the declared return type.Add missing type imports and parameter/return annotations to
Catalog, and align the filesystem, JDBC and REST implementations. Also align the optional database-property type throughRESTApi.create_database()andCreateDatabaseRequest.With these declarations, callers can resolve
Table, read/write builders, snapshots, partitions and tags. The patch covers six files and preserves the existing operation bodies apart from moving imports to module scope.Tests
Catalogmethod annotations withtyping.get_type_hints().None, empty and populated properties. Flake8 andgit diff --checkpassed.