feat: add AqoraCatalog for reading data catalogs via Iceberg - #204
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 31 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds an Aqora Iceberg REST catalog, CLI-based token authentication, custom fsspec filesystem routing, signed URL reads, ranged access, metadata discovery, package entry-point registration, and an optional Iceberg dependency set. ChangesAqora Iceberg integration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PyIceberg
participant AqoraCatalog
participant AqoraAuthManager
participant AqoraIcebergFileSystem
participant AqoraREST
participant ObjectStorage
PyIceberg->>AqoraCatalog: create catalog and request file access
AqoraCatalog->>AqoraAuthManager: obtain cached Bearer token
PyIceberg->>AqoraIcebergFileSystem: read Iceberg metadata or data file
AqoraIcebergFileSystem->>AqoraREST: request signed URL
AqoraREST-->>AqoraIcebergFileSystem: redirect with signed Location
AqoraIcebergFileSystem->>ObjectStorage: GET signed URL with optional Range
ObjectStorage-->>AqoraIcebergFileSystem: bytes and response headers
AqoraIcebergFileSystem-->>PyIceberg: return file data or metadata
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pyproject.toml`:
- Around line 37-40: Update the pyiceberg dependency constraint in the iceberg
extras to require version 0.10.0 or newer, ensuring the AUTH_MANAGER usage in
aqora.iceberg is supported; remove the redundant python_version marker if it is
no longer needed.
In `@python/aqora/iceberg_fs.py`:
- Around line 100-121: Update _info so it raises an explicit error when the
ranged response has no usable Content-Range total and no valid Content-Length,
instead of returning size 0. Preserve the existing size parsing and file
metadata for responses that provide either value.
- Around line 61-78: Update the async _resolve_signed_url method to obtain the
Authorization header by offloading self._auth.auth_header() to a worker thread
via the project’s asyncio-compatible thread helper, then use that result in the
request headers. Keep AqoraAuthManager.auth_header synchronous and preserve the
existing redirect and error handling.
In `@python/aqora/iceberg.py`:
- Around line 58-84: Update _fetch_token to catch subprocess.TimeoutExpired from
subprocess.run and convert it into a clear RuntimeError, preserving the existing
exception chaining and consistent handling alongside FileNotFoundError and
CalledProcessError.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8bf50c30-8981-454e-a868-243eb6dc3e7f
📒 Files selected for processing (5)
.gitignorepyproject.tomlpython/aqora/fsspec.pypython/aqora/iceberg.pypython/aqora/iceberg_fs.py
Adds a PyIceberg `RestCatalog` subclass that talks to the platform's
data-catalog REST service at `/data-catalog/v1/{owner}/{slug}`.
- `AqoraCatalog` overrides `url()` to prefix every endpoint, including
`/config`, because the service is per-catalog rather than per-warehouse.
- `AqoraAuthManager` sources bearer tokens from `aqora auth token`,
re-invoking the CLI after a TTL to pick up rotated tokens.
- `AqoraIcebergFileSystem` resolves the `aqora-iceberg://` URIs embedded in
generated Iceberg metadata, following the service's redirect manually so
the Authorization header is dropped before hitting the presigned S3 URL.
Gated behind a new `iceberg` extra.
417b029 to
545a413
Compare
1 participant
Summary by CodeRabbit
aqora://andaqora-iceberg://filesystem protocols.