Skip to content

Core/FFI/Go: Add Support for Async and Sync Client Types in FFI#3451

Merged
barshaul merged 2 commits intovalkey-io:mainfrom
barshaul:ffi_sync
Mar 31, 2025
Merged

Core/FFI/Go: Add Support for Async and Sync Client Types in FFI#3451
barshaul merged 2 commits intovalkey-io:mainfrom
barshaul:ffi_sync

Conversation

@barshaul
Copy link
Copy Markdown
Collaborator

@barshaul barshaul commented Mar 25, 2025

✨ Summary

This PR introduces support for both asynchronous and synchronous clients across the Rust-C FFI boundary. It unifies command execution and error handling while allowing consuming languages to choose the interaction model that fits their needs. This PR establishes the groundwork required to support synchronous APIs in the Python wrapper.

✅ Key Changes

🔹 Rust (FFI Layer)

  • Added ClientType enum (AsyncClient and SyncClient) to differentiate client behavior.
  • Introduced CommandResult and CommandError to represent sync command results and failures.
  • Updated FFI functions (command, request_cluster_scan, update_connection_password) to return *mut CommandResult for sync support to immediately return result.

🔹 Go

  • Updated createClient to pass the encoded ClientType to the Rust layer.

🔹 Tests

  • Validated behavior for both client types via shared test logic.

🧠 Motivation

  • Allows consuming languages like Go or Python to choose between blocking (sync) and callback-based (async) client interactions.

Issue link

This Pull Request is linked to issue (URL): #3457

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one issue.
  • Commit message has a detailed description of what changed and why.
  • Tests are added or updated.
  • CHANGELOG.md and documentation files are updated.
  • Destination branch is correct - main or release
  • Create merge commit if merging release branch into main, squash otherwise.

@barshaul barshaul force-pushed the ffi_sync branch 4 times, most recently from bc4c45c to e5fc4d1 Compare March 25, 2025 16:43
@barshaul barshaul changed the title Go/Core/FFI: Modify the FFI C client to have async and sync types Go/Core/FFI: Add Support for Async and Sync Client Types in FFI Mar 25, 2025
@barshaul barshaul marked this pull request as ready for review March 26, 2025 12:49
@barshaul barshaul requested a review from a team as a code owner March 26, 2025 12:49
@barshaul barshaul changed the title Go/Core/FFI: Add Support for Async and Sync Client Types in FFI Core/FFI/Go: Add Support for Async and Sync Client Types in FFI Mar 26, 2025
@barshaul barshaul force-pushed the ffi_sync branch 2 times, most recently from eedd057 to badc167 Compare March 26, 2025 12:58
@Yury-Fridlyand Yury-Fridlyand added the go 🏃 golang wrapper label Mar 29, 2025
@Yury-Fridlyand
Copy link
Copy Markdown

Keep in mind that go client is completely sync (even though it uses async backend from glide core).

@avifenesh
Copy link
Copy Markdown
Member

avifenesh commented Mar 29, 2025

Keep in mind that go client is completely sync (even though it uses async backend from glide core).

Can you explain the meaning in this context? Do you mean that there's no callbacks?

@Yury-Fridlyand
Copy link
Copy Markdown

This change is no-op for go client. It is sync already.
There are no plans to rewrite it using sync core API neither make an async implementation.

@barshaul barshaul requested a review from eifrah-aws March 30, 2025 09:08
Signed-off-by: barshaul <barshaul@amazon.com>
Signed-off-by: barshaul <barshaul@amazon.com>
@barshaul barshaul merged commit b5a3378 into valkey-io:main Mar 31, 2025
22 checks passed
@barshaul barshaul deleted the ffi_sync branch March 31, 2025 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go 🏃 golang wrapper

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants