You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iggy.apache.org by "spetz (via GitHub)" <gi...@apache.org> on 2026/07/27 12:50:45 UTC
[PR] feat(sdk): let raw binary requests declare their replication kind [iggy]
spetz opened a new pull request, #3755:
URL: https://github.com/apache/iggy/pull/3755
The raw command path only accepted codes the SDK already knew.
Under VSR the encoder classified every code from a closed table
and refused anything else before any bytes left the client, so
the raw API was not actually open.
Let the caller declare how a raw request executes. One deliberate
API break across all nine SDKs now, so the surface stays fixed
when native VSR framing lands per SDK later.
The declaration never reaches the wire on classic framing, and
tests in every SDK pin the frames as byte-identical. Under VSR
the protocol tables stay authoritative. A shipped code keeps its
class, an unknown non-replicated code is forwarded for the server
to judge, and an unknown replicated code fails until a replicated
extension registry exists. Login and logout are rejected before
any I/O on every transport.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(sdk): let raw binary requests declare their replication kind [iggy]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #3755:
URL: https://github.com/apache/iggy/pull/3755#issuecomment-5091548097
## [Codecov](https://app.codecov.io/gh/apache/iggy/pull/3755?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 73.88%. Comparing base ([`c5fec1f`](https://app.codecov.io/gh/apache/iggy/commit/c5fec1ff646e7513619c771cea909188526a7141?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`01c3b99`](https://app.codecov.io/gh/apache/iggy/commit/01c3b99f693546f3fcc9c97bd09c840aa0af785e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #3755 +/- ##
============================================
- Coverage 74.54% 73.88% -0.67%
+ Complexity 969 968 -1
============================================
Files 1306 1304 -2
Lines 150028 149790 -238
Branches 125462 125359 -103
============================================
- Hits 111840 110668 -1172
- Misses 34686 35620 +934
Partials 3502 3502
```
| [Components](https://app.codecov.io/gh/apache/iggy/pull/3755/components?src=pr&el=components&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [Rust Core](https://app.codecov.io/gh/apache/iggy/pull/3755/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `74.84% <ø> (-0.01%)` | :arrow_down: |
| [Java SDK](https://app.codecov.io/gh/apache/iggy/pull/3755/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `62.70% <ø> (-0.01%)` | :arrow_down: |
| [C# SDK](https://app.codecov.io/gh/apache/iggy/pull/3755/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `72.27% <ø> (-0.01%)` | :arrow_down: |
| [Python SDK](https://app.codecov.io/gh/apache/iggy/pull/3755/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `92.26% <ø> (-0.01%)` | :arrow_down: |
| [PHP SDK](https://app.codecov.io/gh/apache/iggy/pull/3755/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `84.47% <ø> (-0.05%)` | :arrow_down: |
| [Node SDK](https://app.codecov.io/gh/apache/iggy/pull/3755/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `80.83% <100.00%> (-11.42%)` | :arrow_down: |
| [Go SDK](https://app.codecov.io/gh/apache/iggy/pull/3755/components?src=pr&el=component&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.06% <ø> (-0.03%)` | :arrow_down: |
| [Files with missing lines](https://app.codecov.io/gh/apache/iggy/pull/3755?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [core/sdk/src/clients/client.rs](https://app.codecov.io/gh/apache/iggy/pull/3755?src=pr&el=tree&filepath=core%2Fsdk%2Fsrc%2Fclients%2Fclient.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zZGsvc3JjL2NsaWVudHMvY2xpZW50LnJz) | `89.04% <ø> (-0.18%)` | :arrow_down: |
| [core/sdk/src/quic/quic\_client.rs](https://app.codecov.io/gh/apache/iggy/pull/3755?src=pr&el=tree&filepath=core%2Fsdk%2Fsrc%2Fquic%2Fquic_client.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zZGsvc3JjL3F1aWMvcXVpY19jbGllbnQucnM=) | `73.06% <ø> (-0.05%)` | :arrow_down: |
| [core/sdk/src/tcp/tcp\_client.rs](https://app.codecov.io/gh/apache/iggy/pull/3755?src=pr&el=tree&filepath=core%2Fsdk%2Fsrc%2Ftcp%2Ftcp_client.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zZGsvc3JjL3RjcC90Y3BfY2xpZW50LnJz) | `74.57% <ø> (-0.05%)` | :arrow_down: |
| [core/sdk/src/websocket/websocket\_client.rs](https://app.codecov.io/gh/apache/iggy/pull/3755?src=pr&el=tree&filepath=core%2Fsdk%2Fsrc%2Fwebsocket%2Fwebsocket_client.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Y29yZS9zZGsvc3JjL3dlYnNvY2tldC93ZWJzb2NrZXRfY2xpZW50LnJz) | `71.66% <ø> (-0.07%)` | :arrow_down: |
| [foreign/csharp/Iggy\_SDK/IggyClient/IIggyClient.cs](https://app.codecov.io/gh/apache/iggy/pull/3755?src=pr&el=tree&filepath=foreign%2Fcsharp%2FIggy_SDK%2FIggyClient%2FIIggyClient.cs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Zm9yZWlnbi9jc2hhcnAvSWdneV9TREsvSWdneUNsaWVudC9JSWdneUNsaWVudC5jcw==) | `0.00% <ø> (ø)` | |
| [...DK/IggyClient/Implementations/HttpMessageStream.cs](https://app.codecov.io/gh/apache/iggy/pull/3755?src=pr&el=tree&filepath=foreign%2Fcsharp%2FIggy_SDK%2FIggyClient%2FImplementations%2FHttpMessageStream.cs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Zm9yZWlnbi9jc2hhcnAvSWdneV9TREsvSWdneUNsaWVudC9JbXBsZW1lbnRhdGlvbnMvSHR0cE1lc3NhZ2VTdHJlYW0uY3M=) | `74.43% <ø> (ø)` | |
| [...SDK/IggyClient/Implementations/TcpMessageStream.cs](https://app.codecov.io/gh/apache/iggy/pull/3755?src=pr&el=tree&filepath=foreign%2Fcsharp%2FIggy_SDK%2FIggyClient%2FImplementations%2FTcpMessageStream.cs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Zm9yZWlnbi9jc2hhcnAvSWdneV9TREsvSWdneUNsaWVudC9JbXBsZW1lbnRhdGlvbnMvVGNwTWVzc2FnZVN0cmVhbS5jcw==) | `78.07% <ø> (-0.03%)` | :arrow_down: |
| [foreign/go/client/tcp/tcp\_core.go](https://app.codecov.io/gh/apache/iggy/pull/3755?src=pr&el=tree&filepath=foreign%2Fgo%2Fclient%2Ftcp%2Ftcp_core.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Zm9yZWlnbi9nby9jbGllbnQvdGNwL3RjcF9jb3JlLmdv) | `59.72% <ø> (-0.23%)` | :arrow_down: |
| [...ache/iggy/client/async/tcp/AsyncIggyTcpClient.java](https://app.codecov.io/gh/apache/iggy/pull/3755?src=pr&el=tree&filepath=foreign%2Fjava%2Fjava-sdk%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Figgy%2Fclient%2Fasync%2Ftcp%2FAsyncIggyTcpClient.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Zm9yZWlnbi9qYXZhL2phdmEtc2RrL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9pZ2d5L2NsaWVudC9hc3luYy90Y3AvQXN5bmNJZ2d5VGNwQ2xpZW50LmphdmE=) | `96.55% <ø> (ø)` | |
| [...ache/iggy/client/blocking/http/IggyHttpClient.java](https://app.codecov.io/gh/apache/iggy/pull/3755?src=pr&el=tree&filepath=foreign%2Fjava%2Fjava-sdk%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Figgy%2Fclient%2Fblocking%2Fhttp%2FIggyHttpClient.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-Zm9yZWlnbi9qYXZhL2phdmEtc2RrL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9pZ2d5L2NsaWVudC9ibG9ja2luZy9odHRwL0lnZ3lIdHRwQ2xpZW50LmphdmE=) | `84.21% <ø> (ø)` | |
| ... and [5 more](https://app.codecov.io/gh/apache/iggy/pull/3755?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | |
... and [59 files with indirect coverage changes](https://app.codecov.io/gh/apache/iggy/pull/3755/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(sdk): let raw binary requests declare their replication kind [iggy]
Posted by "spetz (via GitHub)" <gi...@apache.org>.
spetz commented on PR #3755:
URL: https://github.com/apache/iggy/pull/3755#issuecomment-5095543421
Ok, fair point, I'll split this PR into one for Rust + Python only and the second one for TS changes. All the other SDKs will follow the changes once VSR framing is implemented in them.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(sdk): let raw binary requests declare their replication kind [iggy]
Posted by "spetz (via GitHub)" <gi...@apache.org>.
spetz closed pull request #3755: feat(sdk): let raw binary requests declare their replication kind
URL: https://github.com/apache/iggy/pull/3755
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(sdk): let raw binary requests declare their replication kind [iggy]
Posted by "hubcio (via GitHub)" <gi...@apache.org>.
hubcio commented on PR #3755:
URL: https://github.com/apache/iggy/pull/3755#issuecomment-5095233711
i'm against this PR as it stands - it's three PRs in one: a breaking API change across nine SDKs, a whole new VSR client for typescript, and an unrelated topic decoder fix.
main issue is the rust SDK isn't stable yet. we're still moving VSR framing around underneath it, so breaking eight other SDKs to match itlocks in an interface for a design that's still in progress.
--
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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(sdk): let raw binary requests declare their replication kind [iggy]
Posted by "slbotbm (via GitHub)" <gi...@apache.org>.
slbotbm commented on code in PR #3755:
URL: https://github.com/apache/iggy/pull/3755#discussion_r3659252567
##########
foreign/cpp/Cargo.toml:
##########
@@ -17,7 +17,7 @@
[package]
name = "iggy-cpp"
-version = "0.1.1"
+version = "0.1.2"
Review Comment:
It would be good to update the bazel version as well.
##########
foreign/cpp/src/client.rs:
##########
@@ -37,6 +37,18 @@ use std::sync::Arc;
/// reserve `u32::MAX` as the sentinel for `partition_id`.
const ANY_PARTITION_ID: u32 = u32::MAX;
+/// A C++ `enum class` can hold any value of its underlying type, so cxx models
+/// a shared enum as an open repr and the catch-all arm is reachable.
+fn resolve_binary_request_kind(
+ kind: ffi::BinaryRequestKind,
+) -> Result<RustBinaryRequestKind, String> {
Review Comment:
Let's convert this to `TryFrom<ffi::BinaryRequestKind> for RustBinaryRequestKind` in type_conversion.rs
##########
bdd/cpp/features/step_definitions/raw_command_steps.cpp:
##########
@@ -25,18 +25,35 @@
#include <cstdint>
#include <exception>
+#include <stdexcept>
#include <string>
#include "world.hpp"
-WHEN("^I send a raw command with code ([0-9]+) and an empty payload$") {
+namespace {
+
+iggy::ffi::BinaryRequestKind parse_request_kind(const std::string &kind) {
+ if (kind == "non_replicated") {
+ return iggy::ffi::BinaryRequestKind::NonReplicated;
+ }
+ if (kind == "replicated") {
+ return iggy::ffi::BinaryRequestKind::Replicated;
+ }
+ throw std::invalid_argument("unknown binary request kind: " + kind);
Review Comment:
Let's inline this since there is only 1 call site.
##########
foreign/cpp/Cargo.toml:
##########
@@ -26,6 +26,12 @@ ignored = ["cxx-build"]
[lib]
crate-type = ["staticlib"]
+# Framing is a compile-time choice, so the static library is locked to whatever it
+# was built with. Opt in with `--features vsr` to build against the consensus
+# protocol; the default build stays on the classic protocol.
+[features]
+vsr = ["iggy/vsr", "iggy_common/vsr"]
Review Comment:
We utilize bazel as the build system, and we'll need to add this feature flag there 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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org