Conversation
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Install Rust | ||
| uses: dtolnay/rust-toolchain@stable |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Install Rust | ||
| uses: dtolnay/rust-toolchain@stable |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
| path: dist | ||
| merge-multiple: true | ||
|
|
||
| - uses: pypa/gh-action-pypi-publish@release/v1 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #84 +/- ##
==========================================
- Coverage 74.89% 74.88% -0.01%
==========================================
Files 63 63
Lines 15024 15024
==========================================
- Hits 11252 11251 -1
- Misses 3772 3773 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR introduces Python FFI bindings for FlowSDK, providing both low-level FFI access and high-level async MQTT client capabilities. The implementation enables Python developers to use FlowSDK's MQTT functionality with support for multiple transport protocols (TCP, TLS, and QUIC).
Changes:
- Added build tooling and CI/CD pipeline for Python bindings generation and PyPI publishing
- Implemented high-level async MQTT client with support for TCP, TLS, and QUIC transports
- Created comprehensive example suite demonstrating various usage patterns (async, select-based, low-level FFI)
- Configured Python package structure with proper packaging metadata
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 35 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/build_python_bindings.sh |
Build script for generating Python bindings from Rust FFI using uniffi |
python/package/setup.py |
Minimal setuptools configuration for package building |
python/package/pyproject.toml |
Package metadata including version, dependencies, and build configuration |
python/package/MANIFEST.in |
Specifies native library files to include in distribution |
python/package/flowsdk/async_client.py |
High-level async MQTT client with protocol implementations for TCP/TLS/QUIC |
python/package/flowsdk/__init__.py |
Package initialization exposing main API components |
python/package/README.md |
User documentation with examples and API reference |
python/examples/test_binding.py |
Low-level FFI binding verification tests |
python/examples/simple_async_usage.py |
Minimal async client example |
python/examples/select_example.py |
Non-blocking I/O example using select() |
python/examples/asyncio_tcp_client_example.py |
TCP transport example with message handling |
python/examples/asyncio_quic_client_example.py |
QUIC transport example |
python/examples/async_tls_client_example.py |
TLS transport example |
python/examples/async_example.py |
Manual asyncio integration with low-level API |
python/examples/README.md |
Comprehensive examples documentation |
.gitignore |
Excludes generated FFI files and Python artifacts |
.github/workflows/release_pypi.yml |
PyPI publishing workflow for releases |
.github/workflows/ci.yml |
Adds Python bindings build and test to CI pipeline |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| async def publish(self, topic: str, payload: bytes, qos: int = 0, retain: Optional[bool] = None) -> int: | ||
| """ | ||
| Publish a message. | ||
|
|
||
| Args: | ||
| topic: MQTT topic to publish to | ||
| payload: Message payload as bytes | ||
| qos: Quality of Service level (0, 1, or 2) | ||
| retain: Whether to retain the message (None uses default, ignored for QUIC) | ||
|
|
||
| Returns: | ||
| Packet ID of the publish (0 for QoS 0) | ||
|
|
||
| Raises: | ||
| RuntimeError: If not connected | ||
|
|
||
| Note: | ||
| QUIC transport does not support the retain parameter and priority. | ||
| """ | ||
| if not self.protocol: | ||
| raise RuntimeError("Not connected") | ||
|
|
||
| loop = asyncio.get_running_loop() | ||
|
|
||
| # Handle different engine publish signatures | ||
| if self.transport_type == TransportType.TCP: | ||
| # TCP engine takes 4 args: topic, payload, qos, priority | ||
| # Note: priority is currently passed as None as it's not exposed in high-level API yet | ||
| pid = self.engine.publish(topic, payload, qos, None) | ||
| else: | ||
| # TLS and QUIC engines take 3 args: topic, payload, qos | ||
| pid = self.engine.publish(topic, payload, qos) |
There was a problem hiding this comment.
The retain parameter in the publish method is accepted but never used. Line 494 documents it as "None uses default, ignored for QUIC" but the parameter isn't passed to any engine's publish method (lines 514 and 517). This creates a misleading API where users might think they can control the retain flag, but it has no effect. Either implement retain flag support or remove it from the API.
| print(f"✅ Published (PID: {ev[0].packet_id})") | ||
| elif ev.is_error(): | ||
| print(f"❌ Error: {ev[0].message}") | ||
| elif ev.is_disconnected(): |
There was a problem hiding this comment.
Inconsistent comment style. Line 164 has a comment describing the disconnected event handling, but line 94 has the comment "Only TCP engine has handle_connection_lost" which seems more like implementation notes. The comment at line 164 should explain what happens when disconnected similar to the explanatory comments for other events, or the comment style should be consistent throughout.
| elif ev.is_disconnected(): | |
| elif ev.is_disconnected(): | |
| # When the engine reports a disconnection, log the reason and | |
| # exit the main loop so the cleanup in the finally block can run. |
| except ImportError: | ||
| pass | ||
|
|
||
| try: | ||
| from .async_client import FlowMqttClient, FlowMqttProtocol, TransportType | ||
| except ImportError: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| except ImportError: | |
| pass | |
| try: | |
| from .async_client import FlowMqttClient, FlowMqttProtocol, TransportType | |
| except ImportError: | |
| except ImportError: | |
| # Optional FFI bindings: allow flowsdk to be imported even if flowsdk_ffi is unavailable. | |
| pass | |
| try: | |
| from .async_client import FlowMqttClient, FlowMqttProtocol, TransportType | |
| except ImportError: | |
| # Optional async MQTT client: expose flowsdk package even when async_client cannot be imported. |
| try: | ||
| asyncio.run(main()) | ||
| except KeyboardInterrupt: | ||
| pass |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| pass | |
| print("\n⏹ Interrupted by user, shutting down...", file=sys.stderr) |
| sent = sock.send(outgoing_data) | ||
| print(f"📤 Sent {sent} bytes") | ||
| outgoing_data = outgoing_data[sent:] | ||
| except (BlockingIOError, InterruptedError): |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| else: | ||
| print("📥 EOF from server") | ||
| return | ||
| except (BlockingIOError, InterruptedError): |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| print(f"📤 Publishing (PID: {pid})...") | ||
| published = True | ||
|
|
||
| except KeyboardInterrupt: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| except KeyboardInterrupt: | |
| except KeyboardInterrupt: | |
| # Allow user interruption (Ctrl+C) and proceed to the cleanup in the finally block. |
with example code.