You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iggy.apache.org by "haze518 (via GitHub)" <gi...@apache.org> on 2025/10/16 16:02:30 UTC
[PR] Add sync client [iggy]
haze518 opened a new pull request, #2274:
URL: https://github.com/apache/iggy/pull/2274
(no 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: commits-unsubscribe@iggy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(sdk): Add sync mode support to Rust SDK [iggy]
Posted by "numinnex (via GitHub)" <gi...@apache.org>.
numinnex commented on PR #2274:
URL: https://github.com/apache/iggy/pull/2274#issuecomment-3414707691
The problem with this approach is the ever growing `ClientWrapper`, that enum starts to explode in complexity, and we would like to start outsourcing it to separate crates, that would use the `sans-io` state machine as protocol translation layer (Request -> Bytes).
Also the PR is gigantic, it's impossible to review, I've barely noticed the `sans-io` part.
Please send just the `sans-io` implementation, without the `sync` client, we don' want to commit into something that would displace our goal to modularize the codebase into smaller components.
--
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): Add sync mode support to Rust SDK [iggy]
Posted by "haze518 (via GitHub)" <gi...@apache.org>.
haze518 commented on PR #2274:
URL: https://github.com/apache/iggy/pull/2274#issuecomment-3414535165
> Thanks for the PR, as discussed here #2132, we would like to migrate our wire protocol to `sans-io` design.
>
> As part of that migration we would like to decouple from the `SDK` crate, all the alternative implementations such as `sync`.
>
> Rather than using the maybe-async crate, which approaches the problem backwards by introducing an additional function color instead of eliminating function coloring, we prefer a state machine, whom job would be to take a Request (like CreateStream) and return raw bytes that the caller can transmit over any network transport of their choice (whether it's sync/async-tokio/udp), those implementations would live outside of the SDK, our SDK would implement only the existing variant which is `async-tokio`.
>
> I appreciate the effort on this PR, however I think we should go back to the sans-io design pattern instead. The current approach doesn't quite fit with the direction we're aiming for in our codebase.
Thanks for the clarification. I completely agree with the direction toward a sansIO design.
The current protocol/core is already runtime-agnostic: it’s responsible only for framing requests (header + payload) and parsing responses, without using any async mechanisms directly. It’s already used in the tcp_sync implementation, which confirms its independence from the execution environment.
It seemed to me that using maybe_async in its current form doesn’t contradict the sansIO approach, since it works purely as a faсade, helping to unify the interface across different wrappers (ClientWrapper, Producer, Consumer) without touching the protocol core itself. Essentially, it’s a compatibility layer rather than a part of the architecture
--
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): Add sync mode support to Rust SDK [iggy]
Posted by "numinnex (via GitHub)" <gi...@apache.org>.
numinnex commented on PR #2274:
URL: https://github.com/apache/iggy/pull/2274#issuecomment-3414488098
Thanks for the PR, as discussed here - https://github.com/apache/iggy/discussions/2132, we would like to migrate our wire protocol to `sans-io` design.
As part of that migration we would like to decouple from the `SDK` crate, all the alternative implementations such as `sync`.
Rather than using the maybe-async crate, which approaches the problem backwards by introducing an additional function color instead of eliminating function coloring, we prefer a state machine, whom job would be to take a Request (like CreateStream) and return raw bytes that the caller can transmit over any network transport of their choice (whether it's sync/async-tokio/udp), those implementations would live outside of the SDK, our SDK would implement only the existing variant which is `async-tokio`.
Thank you for the effort on this PR, however I think we should go back to the sans-io design pattern instead. The current approach doesn't quite fit with the direction we're aiming for in our codebase.
--
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): Add sync mode support to Rust SDK [iggy]
Posted by "haze518 (via GitHub)" <gi...@apache.org>.
haze518 closed pull request #2274: feat(sdk): Add sync mode support to Rust SDK
URL: https://github.com/apache/iggy/pull/2274
--
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): Add sync mode support to Rust SDK [iggy]
Posted by "haze518 (via GitHub)" <gi...@apache.org>.
haze518 commented on PR #2274:
URL: https://github.com/apache/iggy/pull/2274#issuecomment-3414592907
To clarify, the goal wasn’t to hide sync logic behind macros, but to keep a consistent SDK level interface while keeping the core strictly sansIO.
The lower layers (like protocol/core) remain fully decoupled and could be used directly by any runtime implementation
--
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): Add sync mode support to Rust SDK [iggy]
Posted by "haze518 (via GitHub)" <gi...@apache.org>.
haze518 commented on PR #2274:
URL: https://github.com/apache/iggy/pull/2274#issuecomment-3414791364
> Please send just the sans-io implementation, without the sync client, we don' want to commit into something that would displace our goal to modularize the codebase into smaller components.
Sure, I’ll do that
--
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