Skip to content

RUST-2166 Update convenient transactions API to use async closures #1372

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

Merged
merged 7 commits into from
May 9, 2025

Conversation

abr-egn
Copy link
Contributor

@abr-egn abr-egn commented May 8, 2025

RUST-2166

This adds the slightly-awkwardly-named-but-much-easier-to-use and_run2 method, and updates almost all* of the callers to use the new method. It also pulls out the method body into a macro so rather than being duplicated across the async and sync (and now new-async) implementations it's shared across all three.

* A couple of test operations ran into a rustc bug that mean they don't compile with the new method due to really fun implementation of 'std::marker::Send' is not general enough errors, so I left those with the old method. I think it pretty unlikely actual library users will run into this (the trigger condition here is the transaction closure capturing a value that has a manual Send impl when and_run2 is being called inside multiple layers of .box()ed futures AFAICT).

@abr-egn abr-egn marked this pull request as ready for review May 8, 2025 20:32
@abr-egn abr-egn requested a review from a team as a code owner May 8, 2025 20:32
@abr-egn abr-egn requested a review from isabelatkinson May 8, 2025 20:32
@@ -99,6 +99,67 @@ impl<'a> Action for StartTransaction<&'a mut ClientSession> {
}
}

macro_rules! convenient_run {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The logic here is unchanged.

@@ -5,7 +5,7 @@ set -o errexit
source ./.evergreen/env.sh

# Pin clippy to the latest version. This should be updated when new versions of Rust are released.
CLIPPY_VERSION=1.84.0
CLIPPY_VERSION=1.85.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was needed for clippy to accept the async closure syntax.

Copy link
Contributor

@isabelatkinson isabelatkinson left a comment

Choose a reason for hiding this comment

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

the new async closures are so much nicer, lgtm!

@abr-egn abr-egn merged commit 5902d42 into mongodb:main May 9, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants