-
Notifications
You must be signed in to change notification settings - Fork 676
typosquat: Asyncify all the things #9937
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9937 +/- ##
==========================================
- Coverage 89.23% 89.22% -0.01%
==========================================
Files 292 292
Lines 30409 30434 +25
==========================================
+ Hits 27135 27156 +21
- Misses 3274 3278 +4 ☔ View full report in Codecov by Sentry. |
4c167c5 to
e617cd0
Compare
|
☔ The latest upstream changes (presumably #9961) made this pull request unmergeable. Please resolve the merge conflicts. |
55e401e to
10234d5
Compare
|
☔ The latest upstream changes (presumably #9973) made this pull request unmergeable. Please resolve the merge conflicts. |
10234d5 to
221e97f
Compare
221e97f to
4f0e106
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
| let crates = models::Crate::all() | ||
| .inner_join(crate_downloads::table) | ||
| .order(crate_downloads::downloads.desc()) | ||
| .limit(num) | ||
| .load_iter::<models::Crate, DefaultLoadingMode>(conn)? | ||
| { | ||
| let krate = result?; | ||
| crates.insert( | ||
| krate.id, | ||
| ( | ||
| krate.name, | ||
| Crate { | ||
| owners: HashSet::new(), | ||
| }, | ||
| ), | ||
| ); | ||
| } | ||
| .load_stream::<models::Crate>(conn) | ||
| .await? | ||
| .try_fold(crates, |mut crates, krate| { | ||
| crates.insert( | ||
| krate.id, | ||
| ( | ||
| krate.name, | ||
| Crate { | ||
| owners: HashSet::new(), | ||
| }, | ||
| ), | ||
| ); | ||
|
|
||
| futures_util::future::ready(Ok(crates)) | ||
| }) | ||
| .await?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just straight up better, async or no. 👍
... to get rid of these pesky sync database connections.
I wish it would've been possible to split this into multiple commits, but... no 😅