Copilot commented on code in PR #280:
URL: https://github.com/apache/fluss-rust/pull/280#discussion_r2778478892
##########
bindings/python/fluss/__init__.pyi:
##########
@@ -345,21 +348,37 @@ class UpsertWriter:
Args:
row: Dictionary mapping field names to values, or
list/tuple of values in schema order
+
+ Returns:
+ WriteResultHandle: Ignore for fire-and-forget, or call wait() for
ack.
"""
...
- async def delete(self, pk: dict | list | tuple) -> None:
+ def delete(self, pk: dict | list | tuple) -> "WriteResultHandle":
"""Delete a row from the table by primary key.
Args:
pk: Dictionary with PK column names as keys, or
list/tuple of PK values in PK column order
+
+ Returns:
+ WriteResultHandle: Ignore for fire-and-forget, or call wait() for
ack.
Review Comment:
The Returns docstring says to "call wait() for ack", but
`WriteResultHandle.wait()` is async. Update wording to indicate awaiting the
handle (e.g., `await handle.wait()`).
##########
bindings/python/fluss/__init__.pyi:
##########
@@ -301,13 +301,16 @@ class FlussTable:
def __repr__(self) -> str: ...
class AppendWriter:
- async def append(self, row: dict | list | tuple) -> None:
+ def append(self, row: dict | list | tuple) -> "WriteResultHandle":
"""Append a single row to the table.
Args:
row: Dictionary mapping field names to values, or
list/tuple of values in schema order
+ Returns:
+ WriteResultHandle: Ignore for fire-and-forget, or call wait() for
ack.
+
Supported Types:
Currently supports primitive types only:
- Boolean, TinyInt, SmallInt, Int, BigInt (integers)
Review Comment:
`AppendWriter.write_arrow_batch` is still typed as returning `None` in this
stub, but the binding returns a `WriteResultHandle` (so callers can optionally
await per-batch ack). Update the stub signature to return `WriteResultHandle`
to match the actual API.
##########
bindings/python/fluss/__init__.pyi:
##########
@@ -301,13 +301,16 @@ class FlussTable:
def __repr__(self) -> str: ...
class AppendWriter:
- async def append(self, row: dict | list | tuple) -> None:
+ def append(self, row: dict | list | tuple) -> "WriteResultHandle":
"""Append a single row to the table.
Args:
row: Dictionary mapping field names to values, or
list/tuple of values in schema order
+ Returns:
+ WriteResultHandle: Ignore for fire-and-forget, or call wait() for
ack.
Review Comment:
The Returns docstring says to "call wait() for ack", but
`WriteResultHandle.wait()` is async. Wording should indicate awaiting (e.g.,
`await handle.wait()`) to avoid misleading users.
```suggestion
WriteResultHandle: Ignore for fire-and-forget, or await
handle.wait() for acknowledgement.
```
##########
bindings/python/fluss/__init__.pyi:
##########
@@ -345,21 +348,37 @@ class UpsertWriter:
Args:
row: Dictionary mapping field names to values, or
list/tuple of values in schema order
+
+ Returns:
+ WriteResultHandle: Ignore for fire-and-forget, or call wait() for
ack.
"""
...
- async def delete(self, pk: dict | list | tuple) -> None:
+ def delete(self, pk: dict | list | tuple) -> "WriteResultHandle":
"""Delete a row from the table by primary key.
Args:
pk: Dictionary with PK column names as keys, or
list/tuple of PK values in PK column order
+
+ Returns:
+ WriteResultHandle: Ignore for fire-and-forget, or call wait() for
ack.
"""
...
async def flush(self) -> None:
"""Flush all pending upsert/delete operations to the server."""
...
def __repr__(self) -> str: ...
+
+class WriteResultHandle:
+ """Handle for a pending write (append/upsert/delete). Ignore for
fire-and-forget, or await wait() for ack."""
+
Review Comment:
`WriteResultHandle` docstring says "await wait()" which is
ambiguous/misleading (it reads like calling a free function). Consider
clarifying usage as `handle = writer.append(...); await handle.wait()`.
##########
bindings/python/fluss/__init__.pyi:
##########
@@ -301,13 +301,16 @@ class FlussTable:
def __repr__(self) -> str: ...
class AppendWriter:
- async def append(self, row: dict | list | tuple) -> None:
+ def append(self, row: dict | list | tuple) -> "WriteResultHandle":
"""Append a single row to the table.
Review Comment:
The stub currently declares `AppendWriter.flush` as a synchronous `def
flush(self) -> None`, but the actual binding returns an awaitable (implemented
via `future_into_py`) and is used as `await append_writer.flush()` in the
shipped example. Please update the .pyi to `async def flush(self) -> None` so
typing matches runtime behavior.
##########
bindings/python/fluss/__init__.pyi:
##########
@@ -345,21 +348,37 @@ class UpsertWriter:
Args:
row: Dictionary mapping field names to values, or
list/tuple of values in schema order
+
+ Returns:
+ WriteResultHandle: Ignore for fire-and-forget, or call wait() for
ack.
Review Comment:
The Returns docstring says to "call wait() for ack", but
`WriteResultHandle.wait()` is async. Update wording to indicate awaiting the
handle (e.g., `await handle.wait()`).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
