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

Replace async_once with tokio::sync::OnceCell #992

Merged
merged 1 commit into from
Dec 1, 2023
Merged

Replace async_once with tokio::sync::OnceCell #992

merged 1 commit into from
Dec 1, 2023

Conversation

Expyron
Copy link
Contributor

@Expyron Expyron commented Nov 24, 2023

async_once is unmaintained and has been reported unsound.

tokio's parking_lot feature has been added in order to have access to OnceCell::const_new(). This is only necessary in older tokio versions.

@abr-egn abr-egn self-assigned this Nov 28, 2023
@abr-egn abr-egn self-requested a review November 28, 2023 18:41
@abr-egn
Copy link
Contributor

abr-egn commented Nov 28, 2023

Thank you for contributing this! I've authorized an evergreen run and assuming that passes I'll merge this in.

@abr-egn
Copy link
Contributor

abr-egn commented Nov 29, 2023

It looks like this doesn't compile with the sync feature enabled, and also needs a pass through rustfmt +nightly --unstable-features. Once those are fixed I can merge this :)

@Expyron
Copy link
Contributor Author

Expyron commented Nov 29, 2023

I pushed an update:

  • Ran rustfmt
  • I'm not sure what happened with the sync feature, but I don't think it was the changes in that PR. I tried to fix it by updating the log dependency, which was what caused an issue on my machine. If it fails again, can you share the error message?

@abr-egn
Copy link
Contributor

abr-egn commented Nov 30, 2023

Sorry, forgot that evergreen runs aren't publicly visible! Running cargo check --tests --features tokio-sync will reproduce the errors:

error[E0618]: expected function, found `get_client_options`
  --> src/sync/test.rs:64:19
   |
39 | / lazy_static! {
40 | |     static ref get_client_options: ClientOptions =
41 | |         runtime::block_on(async { crate::test::get_client_options().await.clone() });
42 | | }
   | |_- `sync::test::get_client_options` defined here
...
64 |       let options = get_client_options().clone();
   |                     ^^^^^^^^^^^^^^^^^^--
   |                     |
   |                     call expression requires function

... etc ...

@Expyron
Copy link
Contributor Author

Expyron commented Nov 30, 2023

Pushed an update fixing the sync tests

Copy link
Contributor

@abr-egn abr-egn left a comment

Choose a reason for hiding this comment

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

LGTM! Tagging in Isabel.

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.

lgtm, thanks for your contribution!

@abr-egn abr-egn merged commit e02bb47 into mongodb:main Dec 1, 2023
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.

3 participants