Skip to content
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

refactor!: avoid hard dep to tokio rt #4061

Merged
merged 8 commits into from
Jan 24, 2024

Conversation

tisonkun
Copy link
Member

@tisonkun tisonkun commented Jan 23, 2024

This closes #3948.

I'm trying to opt-out tokio deps as much as possible. But it surprises me that simply remove "rt" doesn't give compile error locally. Check CI workflow for more infos.

Signed-off-by: tison <wander4096@gmail.com>
Signed-off-by: tison <wander4096@gmail.com>
@Xuanwo
Copy link
Member

Xuanwo commented Jan 23, 2024

We need to make blocking layer under a feature gate instead. This is a breaking change.

@tisonkun
Copy link
Member Author

@Xuanwo I have an alternative idea.

I'm investigating pollster::block_on to replace tokio runtime. It's a trivial "runtime" to block on the current thread exactly. But I noticed that we have a few features that depends on a running tokio runtime (e.g., pg services), so we may still depends on tokio runtime for running a compatible blocking layer.

@Xuanwo
Copy link
Member

Xuanwo commented Jan 23, 2024

I'm investigating pollster::block_on to replace tokio runtime.

Replacing tokio this way doesn't look good to me. I like the idea of using pollster, but I wish it to be a new layer like PollsterLayer instead.

In this way, our users can decide which runtime to use by switching layers in the next version.

@tisonkun
Copy link
Member Author

so we may still depends on tokio runtime for running a compatible blocking layer.

This is the case. Switching to a feature flag solution.

@tisonkun tisonkun force-pushed the drop-force-tokio-rt-dep branch 2 times, most recently from d8313ea to 7d885c5 Compare January 23, 2024 17:49
@tisonkun tisonkun requested a review from PsiACE January 23, 2024 17:57
@tisonkun
Copy link
Member Author

@Xuanwo @PsiACE seems ready for review now.

@tisonkun tisonkun force-pushed the drop-force-tokio-rt-dep branch 3 times, most recently from 659142e to b9f161e Compare January 23, 2024 18:57
Signed-off-by: tison <wander4096@gmail.com>
core/src/raw/tokio_util.rs Outdated Show resolved Hide resolved
bindings/c/include/opendal.h Show resolved Hide resolved
bindings/java/Cargo.toml Outdated Show resolved Hide resolved
bindings/nodejs/Cargo.toml Outdated Show resolved Hide resolved
bindings/python/Cargo.toml Outdated Show resolved Hide resolved
Signed-off-by: tison <wander4096@gmail.com>
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM, the only thing here is to add a upgrade entry in UPGRADE.md since this is a breaking change.

core/src/raw/tests/utils.rs Outdated Show resolved Hide resolved
@Xuanwo Xuanwo changed the title refactor: avoid hard dep to tokio rt refactor!: avoid hard dep to tokio rt Jan 24, 2024
Signed-off-by: tison <wander4096@gmail.com>
Copy link
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Xuanwo Xuanwo merged commit 21c5e4d into apache:main Jan 24, 2024
196 of 197 checks passed
@tisonkun tisonkun deleted the drop-force-tokio-rt-dep branch January 24, 2024 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make blocking layer optional
5 participants