From b04509cda654e9eac10d55964ccddda286d4ddc1 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sun, 6 Oct 2024 18:59:22 +0200 Subject: [PATCH 1/5] admin/delete_version: Move db connection call outside of `spawn_blocking()` callback --- src/admin/delete_version.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/admin/delete_version.rs b/src/admin/delete_version.rs index 1e004cc569e..65e23da7346 100644 --- a/src/admin/delete_version.rs +++ b/src/admin/delete_version.rs @@ -6,6 +6,7 @@ use crate::worker::jobs; use crate::{admin::dialoguer, db, schema::versions}; use anyhow::Context; use diesel::prelude::*; +use diesel_async::async_connection_wrapper::AsyncConnectionWrapper; #[derive(clap::Parser, Debug)] #[command( @@ -27,10 +28,14 @@ pub struct Opts { } pub async fn run(opts: Opts) -> anyhow::Result<()> { + let conn = db::oneoff_async_connection() + .await + .context("Failed to establish database connection")?; + spawn_blocking(move || { - let crate_name = &opts.crate_name; + let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); - let conn = &mut db::oneoff_connection().context("Failed to establish database connection")?; + let crate_name = &opts.crate_name; let store = Storage::from_environment(); From e4ec9777f97b11a4b671c9dc67a59af579ccf8ba Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sun, 6 Oct 2024 19:09:11 +0200 Subject: [PATCH 2/5] admin/delete_version: Move storage interaction outside of `spawn_blocking()` callback --- src/admin/delete_version.rs | 43 ++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/src/admin/delete_version.rs b/src/admin/delete_version.rs index 65e23da7346..ad4712bd30f 100644 --- a/src/admin/delete_version.rs +++ b/src/admin/delete_version.rs @@ -32,13 +32,13 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> { .await .context("Failed to establish database connection")?; - spawn_blocking(move || { + let store = Storage::from_environment(); + + let opts = spawn_blocking::<_, _, anyhow::Error>(move || { let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); let crate_name = &opts.crate_name; - let store = Storage::from_environment(); - let crate_id: i32 = crates::table .select(crates::id) .filter(crates::name.eq(crate_name)) @@ -53,7 +53,7 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> { println!(); if !opts.yes && !dialoguer::confirm("Do you want to permanently delete these versions?")? { - return Ok(()); + return Ok(opts); } conn.transaction(|conn| { @@ -92,27 +92,26 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> { warn!(%crate_name, ?error, "Failed to enqueue index sync jobs"); } - let rt = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build() - .context("Failed to initialize tokio runtime")?; + Ok(opts) + }).await?; - for version in &opts.versions { - debug!(%crate_name, %version, "Deleting crate file from S3"); - if let Err(error) = rt.block_on(store.delete_crate_file(crate_name, version)) { - warn!(%crate_name, %version, ?error, "Failed to delete crate file from S3"); - } + let crate_name = &opts.crate_name; - debug!(%crate_name, %version, "Deleting readme file from S3"); - match rt.block_on(store.delete_readme(crate_name, version)) { - Err(object_store::Error::NotFound { .. }) => {} - Err(error) => { - warn!(%crate_name, %version, ?error, "Failed to delete readme file from S3") - } - Ok(_) => {} + for version in &opts.versions { + debug!(%crate_name, %version, "Deleting crate file from S3"); + if let Err(error) = store.delete_crate_file(crate_name, version).await { + warn!(%crate_name, %version, ?error, "Failed to delete crate file from S3"); + } + + debug!(%crate_name, %version, "Deleting readme file from S3"); + match store.delete_readme(crate_name, version).await { + Err(object_store::Error::NotFound { .. }) => {} + Err(error) => { + warn!(%crate_name, %version, ?error, "Failed to delete readme file from S3") } + Ok(_) => {} } + } - Ok(()) - }).await + Ok(()) } From 0c435b8eeae2543df8b2630983d6cd2d6d483dc1 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sun, 6 Oct 2024 19:12:05 +0200 Subject: [PATCH 3/5] admin/delete_version: Move crate query outside of `spawn_blocking()` callback --- src/admin/delete_version.rs | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/admin/delete_version.rs b/src/admin/delete_version.rs index ad4712bd30f..34b02a24000 100644 --- a/src/admin/delete_version.rs +++ b/src/admin/delete_version.rs @@ -5,7 +5,7 @@ use crate::tasks::spawn_blocking; use crate::worker::jobs; use crate::{admin::dialoguer, db, schema::versions}; use anyhow::Context; -use diesel::prelude::*; +use diesel::{Connection, ExpressionMethods, QueryDsl}; use diesel_async::async_connection_wrapper::AsyncConnectionWrapper; #[derive(clap::Parser, Debug)] @@ -28,23 +28,30 @@ pub struct Opts { } pub async fn run(opts: Opts) -> anyhow::Result<()> { - let conn = db::oneoff_async_connection() + let mut conn = db::oneoff_async_connection() .await .context("Failed to establish database connection")?; let store = Storage::from_environment(); + let crate_id: i32 = { + use diesel_async::RunQueryDsl; + + crates::table + .select(crates::id) + .filter(crates::name.eq(&opts.crate_name)) + .first(&mut conn) + .await + .context("Failed to look up crate id from the database") + }?; + let opts = spawn_blocking::<_, _, anyhow::Error>(move || { + use diesel::RunQueryDsl; + let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); let crate_name = &opts.crate_name; - let crate_id: i32 = crates::table - .select(crates::id) - .filter(crates::name.eq(crate_name)) - .first(conn) - .context("Failed to look up crate id from the database")?; - println!("Deleting the following versions of the `{crate_name}` crate:"); println!(); for version in &opts.versions { From bc300e41bf4329511f0eab32694ccd45e308fd4d Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sun, 6 Oct 2024 19:17:19 +0200 Subject: [PATCH 4/5] dialoguer: Implement `async_confirm()` fn --- src/admin/dialoguer.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/admin/dialoguer.rs b/src/admin/dialoguer.rs index 9701e6fb3ca..941fe5492bf 100644 --- a/src/admin/dialoguer.rs +++ b/src/admin/dialoguer.rs @@ -1,6 +1,12 @@ +use crate::tasks::spawn_blocking; use ::dialoguer::{theme::Theme, Confirm}; -pub fn confirm(msg: &str) -> dialoguer::Result { +pub async fn async_confirm(msg: impl Into) -> anyhow::Result { + let msg = msg.into(); + spawn_blocking(move || confirm(msg).map_err(anyhow::Error::from)).await +} + +pub fn confirm(msg: impl Into) -> dialoguer::Result { Confirm::with_theme(&CustomTheme) .with_prompt(msg) .default(false) From 9d1f7c3af1f4c1269e09d01d8c9685ba8cb64503 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sun, 6 Oct 2024 19:17:35 +0200 Subject: [PATCH 5/5] admin/delete_version: Move user confirmation outside of `spawn_blocking()` callback --- src/admin/delete_version.rs | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/admin/delete_version.rs b/src/admin/delete_version.rs index 34b02a24000..16665d4f930 100644 --- a/src/admin/delete_version.rs +++ b/src/admin/delete_version.rs @@ -45,11 +45,7 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> { .context("Failed to look up crate id from the database") }?; - let opts = spawn_blocking::<_, _, anyhow::Error>(move || { - use diesel::RunQueryDsl; - - let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); - + { let crate_name = &opts.crate_name; println!("Deleting the following versions of the `{crate_name}` crate:"); @@ -59,9 +55,20 @@ pub async fn run(opts: Opts) -> anyhow::Result<()> { } println!(); - if !opts.yes && !dialoguer::confirm("Do you want to permanently delete these versions?")? { - return Ok(opts); + if !opts.yes + && !dialoguer::async_confirm("Do you want to permanently delete these versions?") + .await? + { + return Ok(()); } + } + + let opts = spawn_blocking::<_, _, anyhow::Error>(move || { + use diesel::RunQueryDsl; + + let conn: &mut AsyncConnectionWrapper<_> = &mut conn.into(); + + let crate_name = &opts.crate_name; conn.transaction(|conn| { info!(%crate_name, %crate_id, versions = ?opts.versions, "Deleting versions from the database");