Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
luoyuxia merged PR #302: URL: https://github.com/apache/fluss-rust/pull/302 -- 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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
luoyuxia commented on PR #302: URL: https://github.com/apache/fluss-rust/pull/302#issuecomment-3891652922 @leekeiabstraction Do you have further comment. If not, I'm to merge it. -- 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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
fresh-borzoni commented on PR #302: URL: https://github.com/apache/fluss-rust/pull/302#issuecomment-3891511857 @leekeiabstraction Ty for the review, answered, added None code in bindings -- 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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
fresh-borzoni commented on code in PR #302:
URL: https://github.com/apache/fluss-rust/pull/302#discussion_r2799435281
##
bindings/cpp/include/fluss.hpp:
##
@@ -46,6 +46,134 @@ struct UpsertWriter;
struct Lookuper;
} // namespace ffi
+/// Named constants for Fluss API error codes.
+///
+/// Server API errors have error_code > 0 or == -1.
+/// Client-side errors have error_code == CLIENT_ERROR (-2).
+/// These constants match the Rust core FlussError enum and are stable across
protocol versions.
+/// New server error codes work automatically (error_code is a raw int, not a
closed enum) —
+/// these constants are convenience names, not an exhaustive list.
+struct ErrorCode {
+/// Client-side error (not from server API protocol). Check error_message
for details.
+static constexpr int CLIENT_ERROR = -2;
+/// The server experienced an unexpected error when processing the request.
+static constexpr int UNKNOWN_SERVER_ERROR = -1;
+/// The server disconnected before a response was received.
+static constexpr int NETWORK_EXCEPTION = 1;
Review Comment:
well, None means 'no error', so it's like a Rust core sentinel, but we may
add it to match :)
--
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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
leekeiabstraction commented on code in PR #302:
URL: https://github.com/apache/fluss-rust/pull/302#discussion_r2799300765
##
bindings/cpp/include/fluss.hpp:
##
@@ -46,6 +46,134 @@ struct UpsertWriter;
struct Lookuper;
} // namespace ffi
+/// Named constants for Fluss API error codes.
+///
+/// Server API errors have error_code > 0 or == -1.
+/// Client-side errors have error_code == CLIENT_ERROR (-2).
+/// These constants match the Rust core FlussError enum and are stable across
protocol versions.
+/// New server error codes work automatically (error_code is a raw int, not a
closed enum) —
+/// these constants are convenience names, not an exhaustive list.
+struct ErrorCode {
+/// Client-side error (not from server API protocol). Check error_message
for details.
+static constexpr int CLIENT_ERROR = -2;
+/// The server experienced an unexpected error when processing the request.
+static constexpr int UNKNOWN_SERVER_ERROR = -1;
+/// The server disconnected before a response was received.
+static constexpr int NETWORK_EXCEPTION = 1;
Review Comment:
Curious on why we don't have none (0) here?
##
bindings/python/src/error.rs:
##
@@ -18,28 +18,246 @@
use pyo3::exceptions::PyException;
use pyo3::prelude::*;
+/// Error code for client-side errors that did not originate from the server
API protocol.
+/// The value -2 is outside the server API error code range (-1 .. 57+), so it
will never
+/// collide with current or future API codes. Consistent with the CPP binding.
+const CLIENT_ERROR_CODE: i32 = -2;
+
/// Fluss errors
#[pyclass(extends=PyException)]
#[derive(Debug, Clone)]
pub struct FlussError {
#[pyo3(get)]
pub message: String,
+#[pyo3(get)]
+pub error_code: i32,
}
#[pymethods]
impl FlussError {
#[new]
-fn new(message: String) -> Self {
-Self { message }
+#[pyo3(signature = (message, error_code=-2))]
+fn new(message: String, error_code: i32) -> Self {
+Self {
+message,
+error_code,
+}
}
fn __str__(&self) -> String {
-format!("FlussError: {}", self.message)
+if self.error_code != CLIENT_ERROR_CODE {
+format!("FlussError(code={}): {}", self.error_code, self.message)
+} else {
+format!("FlussError: {}", self.message)
+}
}
}
impl FlussError {
pub fn new_err(message: impl ToString) -> PyErr {
-PyErr::new::(message.to_string())
+PyErr::new::((message.to_string(), CLIENT_ERROR_CODE))
+}
+
+/// Create a PyErr from a core Error.
+/// `FlussAPIError` variants carry the server protocol error code directly.
+/// All other error kinds are client-side and use CLIENT_ERROR_CODE.
+pub fn from_core_error(error: &fluss::error::Error) -> PyErr {
+use fluss::error::Error;
+let (msg, code) = match error {
+Error::FlussAPIError { api_error } => (api_error.message.clone(),
api_error.code),
+_ => (error.to_string(), CLIENT_ERROR_CODE),
+};
+PyErr::new::((msg, code))
}
}
+
+/// Named constants for Fluss API error codes.
+///
+/// Server API errors have error_code > 0 or == -1.
+/// Client-side errors have error_code == CLIENT_ERROR (-2).
+/// These constants match the Rust core FlussError enum and are stable across
protocol versions.
+/// New server error codes work automatically (error_code is a raw int, not a
closed enum) —
+/// these constants are convenience names, not an exhaustive list.
+#[pyclass]
+pub struct ErrorCode;
+
+#[pymethods]
+impl ErrorCode {
+/// Client-side error (not from server API protocol). Check the error
message for details.
+#[classattr]
+const CLIENT_ERROR: i32 = -2;
+/// The server experienced an unexpected error when processing the request.
+#[classattr]
+const UNKNOWN_SERVER_ERROR: i32 = -1;
+/// The server disconnected before a response was received.
+#[classattr]
+const NETWORK_EXCEPTION: i32 = 1;
Review Comment:
Similarly here as well
--
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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
fresh-borzoni commented on PR #302: URL: https://github.com/apache/fluss-rust/pull/302#issuecomment-3891308555 @luoyuxia Ty for the review. Addressed Copilot comments -- 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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
Copilot commented on code in PR #302:
URL: https://github.com/apache/fluss-rust/pull/302#discussion_r2798974794
##
bindings/python/README.md:
##
@@ -356,7 +356,7 @@ Same as non-partitioned tables — include partition column
values in each row:
```python
table = await conn.get_table(table_path)
-writer = await table.new_append_writer()
+writer = await table.new_append()
Review Comment:
This partitioned-table writing snippet still treats `new_append()` as an
async `AppendWriter` factory (`await table.new_append()`), but the API now
returns a synchronous `TableAppend` builder. Update to `writer =
table.new_append().create_writer()` (no `await`) before calling
`append()`/`flush()`.
```suggestion
writer = table.new_append().create_writer()
```
##
crates/fluss/src/metadata/table.rs:
##
@@ -506,13 +506,9 @@ impl TableDescriptor {
pub fn replication_factor(&self) -> Result {
self.properties
.get("table.replication.factor")
-.ok_or_else(|| InvalidTableError {
-message: "Replication factor is not set".to_string(),
-})?
+.ok_or_else(|| Error::invalid_table("Replication factor is not
set"))?
.parse()
-.map_err(|_e| InvalidTableError {
-message: "Replication factor can't be convert into
int".to_string(),
-})
+.map_err(|_e| Error::invalid_table("Replication factor can't be
convert into int"))
}
Review Comment:
Typo in error message: "can't be convert into int" → "can't be converted
into int" (or "cannot be converted to an int"). This string is user-facing and
was modified in this PR.
##
bindings/python/src/table.rs:
##
@@ -517,6 +507,141 @@ impl FlussTable {
}
}
+/// Builder for creating an AppendWriter.
+///
+/// Obtain via `FlussTable.new_append()`, then call `create_writer()`.
+#[pyclass]
+pub struct TableAppend {
+inner: fcore::client::TableAppend,
+table_info: fcore::metadata::TableInfo,
+}
+
+#[pymethods]
+impl TableAppend {
+/// Create an AppendWriter from this builder.
+pub fn create_writer(&self) -> PyResult {
+let rust_writer = self
+.inner
+.create_writer()
+.map_err(|e| FlussError::from_core_error(&e))?;
+Ok(AppendWriter::from_core(
+rust_writer,
+self.table_info.clone(),
+))
+}
+
+fn __repr__(&self) -> String {
+"TableAppend()".to_string()
+}
+}
+
+/// Builder for creating an UpsertWriter, with optional partial update
configuration.
+///
+/// Obtain via `FlussTable.new_upsert()`, then optionally call
+/// `partial_update_by_name()` or `partial_update_by_index()`,
+/// then call `create_writer()`.
+#[pyclass]
+pub struct TableUpsert {
+inner: fcore::client::TableUpsert,
+table_info: fcore::metadata::TableInfo,
+/// Column indices for partial updates, tracked for Python's
dict→GenericRow conversion.
+target_columns: Option>,
+}
+
+#[pymethods]
+impl TableUpsert {
+/// Configure partial update by column names.
+///
+/// Only the specified columns will be updated on upsert.
+///
+/// Args:
+/// columns: List of column names to update.
+///
+/// Returns:
+/// A new TableUpsert configured for partial update.
+pub fn partial_update_by_name(&self, columns: Vec) ->
PyResult {
+let col_refs: Vec<&str> = columns.iter().map(|s| s.as_str()).collect();
+// Core validates and resolves names → indices internally
+let updated = self
+.inner
+.partial_update_with_column_names(&col_refs)
+.map_err(|e| FlussError::from_core_error(&e))?;
+// Resolve indices for Python's row conversion layer (core validated
names above)
+let row_type = self.table_info.row_type();
+let indices: Vec = columns
+.iter()
+.map(|name| row_type.get_field_index(name).unwrap())
+.collect();
Review Comment:
`row_type.get_field_index(name).unwrap()` can panic if the mapping ever
diverges (even if core validation changes). Prefer converting the `Option` into
a Python error (`FlussError`) instead of unwrapping.
```suggestion
.map(|name| {
row_type
.get_field_index(name)
.ok_or_else(|| {
FlussError::new_err(format!(
"Unknown column name '{}' for partial update",
name
))
})
})
.collect::>>()?;
```
##
docs/rust-client.md:
##
@@ -454,7 +454,7 @@ let log_scanner = table.new_scan().create_log_scanner()?;
// Subscribe to each partition's buckets
for partition_info in &partitions {
let partition_id = partition_info.get_partition_id();
-le
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
fresh-borzoni commented on code in PR #302: URL: https://github.com/apache/fluss-rust/pull/302#discussion_r2798375578 ## bindings/cpp/src/lib.rs: ## Review Comment: I think we can be fine with ClientError for now, if we need - we can additively add new codes without breaking existing clients. Then -2 becomes the fallback. Or we can handle it separately if you feel it's the right move now, this PR is already becoming quite big :) -- 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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
fresh-borzoni commented on PR #302: URL: https://github.com/apache/fluss-rust/pull/302#issuecomment-3890471728 For retriable errors, we might want to add some marker - this is the real need for IO/Network/Rpc level errors, but I believe it's only for server level errors, no client errors are needed to be retriable for now. So technically should be fine. -- 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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
fresh-borzoni commented on code in PR #302: URL: https://github.com/apache/fluss-rust/pull/302#discussion_r2798375578 ## bindings/cpp/src/lib.rs: ## Review Comment: I think we can be fine with ClientError for now, if we need - we can additively add new codes without breaking existing clients. Then -2 becomes the fallback. For retriable errors, we might want to add some marker - this is the real need for IO/Network/Rpc level errors, but I believe it's only for server level errors, no client errors are needed to be retriable for now. So technically should be fine. Or we can handle it separately if you feel it's the right move now, this PR is already becoming quite big :) -- 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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
fresh-borzoni commented on code in PR #302: URL: https://github.com/apache/fluss-rust/pull/302#discussion_r2798375578 ## bindings/cpp/src/lib.rs: ## Review Comment: I think we can be fine with ClientError for now, if we need - we can additively add new codes without breaking existing clients. Then -2 becomes the fallback. For retriable errors, we might want to add some retriable marker - this is the real need for IO/Network/Rpc level errors in the client. Or we can handle it separately if you feel it's the right move now, this PR is already becoming quite big :) ## bindings/cpp/src/lib.rs: ## Review Comment: I think we can be fine with ClientError for now, if we need - we can additively add new codes without breaking existing clients. Then -2 becomes the fallback. For retriable errors, we might want to add some marker - this is the real need for IO/Network/Rpc level errors in the client. Or we can handle it separately if you feel it's the right move now, this PR is already becoming quite big :) -- 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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
fresh-borzoni commented on code in PR #302: URL: https://github.com/apache/fluss-rust/pull/302#discussion_r2798375578 ## bindings/cpp/src/lib.rs: ## Review Comment: I think we can be fine with ClientError for now, if we need - we can additively add new codes without breaking existing clients. Then -2 becomes the fallback. Or we can handle it separately if you feel it's the right move now, this PR is already becoming quite big :) -- 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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
fresh-borzoni commented on code in PR #302: URL: https://github.com/apache/fluss-rust/pull/302#discussion_r2798375578 ## bindings/cpp/src/lib.rs: ## Review Comment: I think we can be fine with ClientError for now, if we need - we can additively add new codes without breaking existing code. Or we can handle it separately if you feel it's the right move now, this PR is already becoming quite big :) -- 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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
fresh-borzoni commented on code in PR #302: URL: https://github.com/apache/fluss-rust/pull/302#discussion_r2798375578 ## bindings/cpp/src/lib.rs: ## Review Comment: I think we can be fine with ClientError for now, if we need - we can additively add new codes without breaking existing clients. Or we can handle it separately if you feel it's the right move now, this PR is already becoming quite big :) -- 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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
fresh-borzoni commented on PR #302: URL: https://github.com/apache/fluss-rust/pull/302#issuecomment-3890344787 @luoyuxia @leekeiabstraction TY for the review Adressed comments. PTAL 🙏 -- 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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
fresh-borzoni commented on code in PR #302: URL: https://github.com/apache/fluss-rust/pull/302#discussion_r2797914150 ## bindings/cpp/src/lib.rs: ## Review Comment: Hmm, I would avoid constants, client errors should be just some programmatic errors. I'd better restructure it more. -- 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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
fresh-borzoni commented on code in PR #302:
URL: https://github.com/apache/fluss-rust/pull/302#discussion_r2797915851
##
bindings/cpp/src/lib.rs:
##
@@ -451,16 +451,25 @@ fn err_result(code: i32, msg: String) -> ffi::FfiResult {
}
}
+/// Convert a core Error to FfiResult, extracting the API error code when
available.
+fn err_from_core_error(e: &fcore::error::Error) -> ffi::FfiResult {
+if let fcore::error::Error::FlussAPIError { api_error } = e {
+err_result(api_error.code, api_error.message.clone())
+} else {
+err_result(-1, e.to_string())
Review Comment:
Good catch, it's an oversight
--
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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
fresh-borzoni commented on code in PR #302:
URL: https://github.com/apache/fluss-rust/pull/302#discussion_r2797804626
##
bindings/python/example/example.py:
##
@@ -105,7 +105,7 @@ async def main():
print(f"Got table: {table}")
# Create a writer for the table
-append_writer = await table.new_append_writer()
+append_writer = await table.new_append()
Review Comment:
a bit unpythonic, but better to be consistent
--
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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
leekeiabstraction commented on PR #302: URL: https://github.com/apache/fluss-rust/pull/302#issuecomment-3889601145 Thank you for the PR! Left some comments, PTAL -- 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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
leekeiabstraction commented on code in PR #302:
URL: https://github.com/apache/fluss-rust/pull/302#discussion_r2797579392
##
bindings/cpp/src/lib.rs:
##
@@ -451,16 +451,25 @@ fn err_result(code: i32, msg: String) -> ffi::FfiResult {
}
}
+/// Convert a core Error to FfiResult, extracting the API error code when
available.
+fn err_from_core_error(e: &fcore::error::Error) -> ffi::FfiResult {
Review Comment:
Should we also have a constant file like in Rust's `FlussError`? So that
users do not have to maintain their own constants
##
bindings/python/src/error.rs:
##
@@ -24,22 +24,41 @@ use pyo3::prelude::*;
pub struct FlussError {
#[pyo3(get)]
pub message: String,
+#[pyo3(get)]
+pub error_code: i32,
}
#[pymethods]
impl FlussError {
#[new]
-fn new(message: String) -> Self {
-Self { message }
+#[pyo3(signature = (message, error_code=0))]
+fn new(message: String, error_code: i32) -> Self {
+Self {
+message,
+error_code,
+}
}
fn __str__(&self) -> String {
-format!("FlussError: {}", self.message)
+if self.error_code != 0 {
+format!("FlussError(code={}): {}", self.error_code, self.message)
+} else {
+format!("FlussError: {}", self.message)
+}
}
}
impl FlussError {
pub fn new_err(message: impl ToString) -> PyErr {
-PyErr::new::(message.to_string())
+PyErr::new::((message.to_string(), 0i32))
+}
+
+/// Create a PyErr from a core Error, extracting the API error code if
available.
+pub fn from_core_error(error: &fluss::error::Error) -> PyErr {
+if let fluss::error::Error::FlussAPIError { api_error } = error {
+PyErr::new::((api_error.message.clone(),
api_error.code))
+} else {
+PyErr::new::((error.to_string(), 0i32))
Review Comment:
Magic number?
##
bindings/cpp/src/lib.rs:
##
@@ -451,16 +451,25 @@ fn err_result(code: i32, msg: String) -> ffi::FfiResult {
}
}
+/// Convert a core Error to FfiResult, extracting the API error code when
available.
+fn err_from_core_error(e: &fcore::error::Error) -> ffi::FfiResult {
+if let fcore::error::Error::FlussAPIError { api_error } = e {
+err_result(api_error.code, api_error.message.clone())
+} else {
+err_result(-1, e.to_string())
Review Comment:
Wouldn't this cause user to be unable to distinguish non FlussAPIError and
FlussAPIError?
UnknownServerError is mapped to `-1` on Rust and Java side
##
bindings/cpp/src/lib.rs:
##
Review Comment:
Should we consider updating this as well? Actually, I do not understand the
error code used for some of these calls. Most of them are called with hardcoded
1 which maps to API Error's `NetworkException`, I suspect that is incorrect. We
should have error constants for client side exception as well
--
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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
luoyuxia commented on code in PR #302:
URL: https://github.com/apache/fluss-rust/pull/302#discussion_r2797280817
##
bindings/python/example/example.py:
##
@@ -105,7 +105,7 @@ async def main():
print(f"Got table: {table}")
# Create a writer for the table
-append_writer = await table.new_append_writer()
+append_writer = await table.new_append()
Review Comment:
also follow the pattern that
table.new_append().create_writer()?
##
bindings/python/example/example.py:
##
@@ -32,15 +32,15 @@ async def main():
config_spec = {
"bootstrap.servers": "127.0.0.1:9123",
# Add other configuration options as needed
-"request.max.size": "10485760", # 10 MB
+"writer.request-max-size": "10485760", # 10 MB
"writer.acks": "all", # Wait for all replicas to acknowledge
"writer.retries": "3", # Retry up to 3 times on failure
-"writer.batch.size": "1000", # Batch size for writes
+"writer.batch-size": "1000", # Batch size for writes
}
config = fluss.Config(config_spec)
-# Create connection using the static connect method
-conn = await fluss.FlussConnection.connect(config)
+# Create connection using the static create method
+conn = await fluss.FlussConnection.create(config)
Review Comment:
warn from my ide
```
Unresolved attribute reference 'create' for class 'FlussConnection'
```
we will need update `__init__.pyi`.
The same for
`TableScan.create_batch_scanner()`, `FlussTable.new_append_writer()`,
`LogScanner.poll_batches()`.
--
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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
fresh-borzoni commented on PR #302: URL: https://github.com/apache/fluss-rust/pull/302#issuecomment-3888164689 Addressed comment -- 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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
Copilot commented on code in PR #302:
URL: https://github.com/apache/fluss-rust/pull/302#discussion_r2796329652
##
bindings/cpp/src/lib.rs:
##
@@ -451,16 +451,25 @@ fn err_result(code: i32, msg: String) -> ffi::FfiResult {
}
}
+/// Convert a core Error to FfiResult, extracting the API error code when
available.
+fn err_from_core_error(e: &fcore::error::Error) -> ffi::FfiResult {
+if let fcore::error::Error::FlussAPIError { api_error } = e {
+err_result(api_error.code, api_error.message.clone())
+} else {
+err_result(0, e.to_string())
Review Comment:
In `err_from_core_error`, the non-API error branch returns `error_code = 0`,
but in the C++ `Result` type `Ok()` is defined as `error_code == 0`. This will
cause generic/core errors to be treated as success by C++ callers. Use a
non-zero generic error code for the fallback branch (keeping `api_error.code`
for `FlussAPIError`) so failures are never encoded as `0`.
```suggestion
// Use a non-zero generic error code so C++ callers do not treat
this as success.
err_result(-1, e.to_string())
```
--
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]
Re: [PR] [TASK-295] Verify API consistent across clients [fluss-rust]
fresh-borzoni commented on PR #302: URL: https://github.com/apache/fluss-rust/pull/302#issuecomment-3888091123 @luoyuxia @leekeiabstraction PTAL 🙏 -- 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]
