Conversation
|
@fresh-borzoni Appreciate your review here 🙏 |
luoyuxia
left a comment
There was a problem hiding this comment.
@leekeiabstraction Thank you very much for the pr. I'm always think adding test in python binding
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive Python integration tests for the Fluss Python bindings, mirroring the existing Rust integration tests. The implementation uses pytest with async fixtures and testcontainers to automatically spin up a Fluss cluster (ZooKeeper, coordinator, and tablet server) for testing.
Changes:
- Added Python integration test suite covering admin operations, log tables, and KV tables
- Updated documentation to include instructions for running Python integration tests locally
- Added CI workflow to run Python integration tests across Python 3.11, 3.12, and 3.13
- Updated .gitignore to exclude Python test/build artifacts
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| bindings/python/test/conftest.py | pytest fixtures for cluster setup and teardown using testcontainers |
| bindings/python/test/test_admin.py | Integration tests for database and table admin operations, including error handling |
| bindings/python/test/test_log_table.py | Integration tests for append-only log table operations (append, scan, projection, partitioning) |
| bindings/python/test/test_kv_table.py | Integration tests for KV table operations (upsert, delete, lookup, partial updates, data types) |
| bindings/python/pyproject.toml | Added testcontainers dependency and pytest configuration |
| .github/workflows/ci.yml | Added python-integration-test job for CI |
| website/docs/developer-guide/contributing.md | Added Python testing instructions |
| .gitignore | Added .venv/, uv.lock, and .cache/ entries |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
@leekeiabstraction Ty for the PR. Great work
Left minor comments, also some of Copilot comments make sense. PTAL
1fc3da7 to
6b54b71
Compare
|
@fresh-borzoni Thank you for your review! Addressed comments, PTAL |
fresh-borzoni
left a comment
There was a problem hiding this comment.
@leekeiabstraction Ty, LGTM 👍
Purpose
Linked issue: close #339
Brief change log
Tests
Locally ran and verified that tests run correctly
NOTE: The current 2 failures are due to bug which should be fixed once #340 is merged