Skip to content

refactor!: avoid hard dep to tokio rt#4061

Merged
Xuanwo merged 8 commits intoapache:mainfrom
tisonkun:drop-force-tokio-rt-dep
Jan 24, 2024
Merged

refactor!: avoid hard dep to tokio rt#4061
Xuanwo merged 8 commits intoapache:mainfrom
tisonkun:drop-force-tokio-rt-dep

Conversation

@tisonkun
Copy link
Copy Markdown
Member

@tisonkun tisonkun commented Jan 23, 2024 •
edited by Xuanwo
Loading

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.

@tisonkun
refactor: avoid hard dep to tokio rt
8f67c9a 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun tisonkun requested a review from Xuanwo as a code owner January 23, 2024 16:39
@github-actions github-actions bot added the releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor" label Jan 23, 2024
@github-actions github-actions bot requested a review from xyjixyjixyji January 23, 2024 16:40
PsiACE
PsiACE reviewed Jan 23, 2024
View reviewed changes
@tisonkun
replace with pollster::block_on
64362a0 
Signed-off-by: tison <wander4096@gmail.com>
@Xuanwo
Copy link
Copy Markdown
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
Copy Markdown
Member Author

tisonkun commented Jan 23, 2024

@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
Copy Markdown
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
Copy Markdown
Member Author

tisonkun commented Jan 23, 2024

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
Copy Markdown
Member Author

tisonkun commented Jan 23, 2024

@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
@tisonkun
behind a feature flag
9e91d97 
Signed-off-by: tison <wander4096@gmail.com>
@tisonkun tisonkun force-pushed the drop-force-tokio-rt-dep branch from b9f161e to 9e91d97 Compare January 23, 2024 19:07
Xuanwo
Xuanwo reviewed Jan 24, 2024
View reviewed changes
ClSlaid
ClSlaid reviewed Jan 24, 2024
View reviewed changes
@tisonkun
add internal-tokio-rt
a078ee2 
Signed-off-by: tison <wander4096@gmail.com>
Xuanwo
Xuanwo reviewed Jan 24, 2024
View reviewed changes
Copy link
Copy Markdown
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.

@Xuanwo Xuanwo changed the title refactor: avoid hard dep to tokio rt refactor!: avoid hard dep to tokio rt Jan 24, 2024
@tisonkun
improve feature flags
df35ac1 
Signed-off-by: tison <wander4096@gmail.com>
Xuanwo
Xuanwo approved these changes Jan 24, 2024
View reviewed changes
Copy link
Copy Markdown
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
@tisonkun tisonkun deleted the drop-force-tokio-rt-dep branch January 24, 2024 07:15
@morristai morristai mentioned this pull request Jan 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ClSlaid ClSlaid ClSlaid left review comments

@Xuanwo Xuanwo Xuanwo approved these changes

@xyjixyjixyji xyjixyjixyji Awaiting requested review from xyjixyjixyji xyjixyjixyji is a code owner

@messense messense Awaiting requested review from messense messense is a code owner

@Zheaoli Zheaoli Awaiting requested review from Zheaoli Zheaoli is a code owner

@suyanhanx suyanhanx Awaiting requested review from suyanhanx suyanhanx is a code owner

@PsiACE PsiACE Awaiting requested review from PsiACE PsiACE is a code owner

+1 more reviewer

@weihanglo weihanglo weihanglo left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

releases-note/refactor The PR does a refactor on code or has a title that begins with "refactor"

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Make blocking layer optional

5 participants

@tisonkun @Xuanwo @weihanglo @PsiACE @ClSlaid