-
Notifications
You must be signed in to change notification settings - Fork 472
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(bindings/cpp): init the async support of C++ binding #5195
base: main
Are you sure you want to change the base?
Conversation
@@ -34,6 +34,8 @@ crate-type = ["staticlib"] | |||
anyhow = "1.0" | |||
chrono = "0.4" | |||
cxx = "1.0" | |||
# cxx-async v0.1.1 in crates.io has build problems, so we use git repo directly | |||
cxx-async = { git = "https://github.com/pcwalton/cxx-async", rev = "961dd106b8eb2a86991728e1a18948e597426c1a", optional = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It appears that cxx-async is no longer maintained. Should we consider forking it and maintaining it ourselves?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I also noticed that. Good to folk it so that we don't need to wait maintainers response while contributing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may prevent us from merging this PR. OpenDAL does not permit dependencies on a git version crate, as this could disrupt our release process.
I'm trying to contact with @pcwalton at pcwalton/cxx-async#6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Hope the author can respond so we don't need to folk.
It's ready for review now : ) |
auto path = "test_path"; | ||
std::vector<uint8_t> data{1, 2, 3, 4, 5}; | ||
cppcoro::sync_wait(op->write(path, data)); | ||
auto res = cppcoro::sync_wait(op->read(path)); | ||
for (size_t i = 0; i < data.size(); ++i) EXPECT_EQ(data[i], res[i]); | ||
|
||
path = "test_path2"; | ||
cppcoro::sync_wait([&]() -> cppcoro::task<void> { | ||
co_await op->write(path, data); | ||
auto res = co_await op->read(path); | ||
for (size_t i = 0; i < data.size(); ++i) EXPECT_EQ(data[i], res[i]); | ||
co_return; | ||
}()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A quick demo of async C++ binding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So cool!
Which issue does this PR close?
Part of #5194.
Rationale for this change
This PR initializes the async version of C++ binding.
What changes are included in this PR?
write
andread
methodsAre there any user-facing changes?
Since
OPENDAL_ENABLE_ASYNC
is off by default, so current users shouldn't feel the difference.