feat: add compio support for aya-log#1272
Conversation
❌ Deploy Preview for aya-rs-docs failed.Built without sensitive environment variables
|
In previous version, user can enable async_tokio and async_std features, however when both of them are enabled, tokio will be ignored. After add async_compio feature, when two or three async features are enabled, user may confuse which one will be used. IMO the better way is make these async features are mutual exclusivity, user should only enable one, if more than async feautres are enabled, will compile fail with compile_error! macro. Also if none of the async feature is enabled in aya-log, will compile fail too
|
In previous version, user can enable async_tokio and async_std features, however when both of them are enabled, tokio will be ignored. After add async_compio feature, when two or three async features are enabled, user may confuse which one will be used. IMO the better way is make these async features are mutual exclusivity, user should only enable one, if more than async feautres are enabled, will compile fail with compile_error! macro. Also if none of the async feature is enabled in aya-log, will compile fail too |
vadorovsky
left a comment
There was a problem hiding this comment.
Amazing stuff, thanks for your contribution! 🙏
Would you be interested in adding compio support to integration tests? So far they are using only tokio.
I think making this one work with compio would be enough:
aya/test/integration-test/src/tests/log.rs
Lines 41 to 42 in ccf6c47
|
|
||
| // check async feature mutual exclusivity | ||
| #[cfg(all(feature = "async_tokio", feature = "async_std"))] | ||
| compile_error!("Cannot enable both async_tokio and async_std features at the same time"); |
There was a problem hiding this comment.
I think it's a very good change, thanks for doing that! I agree that async* features should be mutually exclusive.
The CI jobs are red, because they use cargo hack with --feature-powerset flag, which tries out all combinations of features. To make it work, you will need to add the
--mutually-exclusive-features async_compio,async_tokio,async_std
flag in the following places:
Lines 59 to 68 in ccf6c47
Lines 98 to 107 in ccf6c47
Lines 109 to 121 in ccf6c47
Lines 123 to 136 in ccf6c47
Lines 171 to 181 in ccf6c47
Lines 183 to 193 in ccf6c47
tamird
left a comment
There was a problem hiding this comment.
Reviewed 8 of 8 files at r1, 4 of 4 files at r2, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Sherlock-Holo)
aya/Cargo.toml line 25 at r1 (raw file):
bitflags = { workspace = true } bytes = { workspace = true } compio = { workspace = true, optional = true, features = ["default"] }
what is "default"? can we just use the features we need?
aya/Cargo.toml line 40 at r1 (raw file):
async_std = ["dep:async-io"] async_tokio = ["tokio/net"] async_compio = ["dep:compio"]
here we should activate the appropriate features of async-global-executor
Code quote:
async_std = ["dep:async-io"]
async_tokio = ["tokio/net"]
async_compio = ["dep:compio"]aya-log/src/lib.rs line 188 at r1 (raw file):
#[cfg(all(not(feature = "async_tokio"), feature = "async_std"))] { async_global_executor::spawn(fut).detach();
we can just always use this, right? we should be selecting the correct features of async-global-executor to handle the different runtimes internally.
Should I add features +[features]
+default = ["async_tokio"]
+async_tokio = ["dep:tokio", "aya-log/async_tokio"]
+async_compio = ["dep:compio", "aya-log/async_compio"]to make this test function work with tokio and compio? Because we make the async features mutual exclusivity |
compio should only enable the runtime feature, and just enable the io-uring feature in aya-log test
yes, we should just enable
yes, we should enable
It seems even we use |
|
I haven't forgotten about this; I think we should first remove the tokio dependency from aya-log so that it can be more-or-less agnostic to the user's chosen executor. I sent #1288 to do this. Regarding mutually-exclusive features, see https://doc.rust-lang.org/cargo/reference/features.html#mutually-exclusive-features. I'm not sure we should do that. Currently in aya we arbitrarily pick tokio over async-io, but we should probably go the other way since async-io spins a thread, so its futures would work with any executor, including tokio. |
|
With #1288 merged, I think we can close this. We are moving away from supporting individual async runtimes. You can use aya-log with compio using https://docs.rs/compio/latest/compio/fs/struct.AsyncFd.html similarly to the manner in which the example code uses tokio. See also #1292. |
|
Closing. Please let us know if your needs remain unmet. |
compio is a completion IO based async runtime, specially it provide io-uring for Linux user
adding compio support can allow user use
aya-login another async runtime, not just tokioThis change is