Skip to content

Skip running a job if the crate/version deleted #11500

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 26 additions & 8 deletions src/worker/jobs/readmes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use diesel_async::AsyncConnection;
use diesel_async::scoped_futures::ScopedFutureExt;
use serde::{Deserialize, Serialize};
use std::sync::Arc;
use tracing::{info, instrument};
use tracing::{info, instrument, warn};

#[derive(Clone, Serialize, Deserialize)]
pub struct RenderAndUploadReadme {
Expand Down Expand Up @@ -70,13 +70,31 @@ impl BackgroundJob for RenderAndUploadReadme {
let mut conn = env.deadpool.get().await?;
conn.transaction(|conn| {
async move {
Version::record_readme_rendering(job.version_id, conn).await?;
let (crate_name, vers): (String, String) = versions::table
.find(job.version_id)
.inner_join(crates::table)
.select((crates::name, versions::num))
.first(conn)
.await?;
let (crate_name, vers): (String, String) =
match Version::record_readme_rendering(job.version_id, conn)
.await
.and(
versions::table
.find(job.version_id)
.inner_join(crates::table)
.select((crates::name, versions::num))
.first(conn)
.await,
) {
Ok(r) => r,
Err(diesel::result::Error::DatabaseError(
diesel::result::DatabaseErrorKind::ForeignKeyViolation,
..,
))
| Err(diesel::result::Error::NotFound) => {
warn!(
"Skipping rendering README for vesion {}: no version found",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
"Skipping rendering README for vesion {}: no version found",
"Skipping rendering README for version {}: no version found",

job.version_id
);
return Ok(());
}
Err(err) => return Err(err.into()),
};

tracing::Span::current().record("krate.name", tracing::field::display(&crate_name));

Expand Down
13 changes: 11 additions & 2 deletions src/worker/jobs/send_publish_notifications.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,12 @@ impl BackgroundJob for SendPublishNotificationsJob {
let mut conn = ctx.deadpool.get().await?;

// Get crate name, version and other publish details
let publish_details = PublishDetails::for_version(version_id, &mut conn).await?;
let Some(publish_details) = PublishDetails::for_version(version_id, &mut conn).await?
else {
warn!("Skipping publish notifications for {version_id}: no version found",);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
warn!("Skipping publish notifications for {version_id}: no version found",);
warn!("Skipping publish notifications for {version_id}: no version found");


return Ok(());
};

let publish_time = publish_details
.publish_time
Expand Down Expand Up @@ -157,13 +162,17 @@ struct PublishDetails {
}

impl PublishDetails {
async fn for_version(version_id: i32, conn: &mut AsyncPgConnection) -> QueryResult<Self> {
async fn for_version(
version_id: i32,
conn: &mut AsyncPgConnection,
) -> QueryResult<Option<Self>> {
versions::table
.find(version_id)
.inner_join(crates::table)
.left_join(users::table)
.select(PublishDetails::as_select())
.first(conn)
.await
.optional()
}
}
14 changes: 11 additions & 3 deletions src/worker/jobs/update_default_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::worker::Environment;
use crates_io_worker::BackgroundJob;
use serde::{Deserialize, Serialize};
use std::sync::Arc;
use tracing::info;
use tracing::{info, warn};

#[derive(Serialize, Deserialize)]
pub struct UpdateDefaultVersion {
Expand All @@ -28,8 +28,16 @@ impl BackgroundJob for UpdateDefaultVersion {

info!("Updating default version for crate {crate_id}");
let mut conn = ctx.deadpool.get().await?;
update_default_version(crate_id, &mut conn).await?;
let res = update_default_version(crate_id, &mut conn).await;
if let Err(diesel::result::Error::DatabaseError(
diesel::result::DatabaseErrorKind::ForeignKeyViolation,
..,
)) = res
{
warn!("Skipping update default version for crate for {crate_id}: no crate found",);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
warn!("Skipping update default version for crate for {crate_id}: no crate found",);
warn!("Skipping update default version for crate for {crate_id}: no crate found");

return Ok(());
}

Ok(())
res.map_err(|e| e.into())
}
}