From ececdf853c2bb30d1539505e45256576d2c0b8eb Mon Sep 17 00:00:00 2001 From: Sean Griffin Date: Mon, 18 Mar 2019 14:15:23 -0600 Subject: [PATCH] Update swirl New features in this version: - Codegen for jobs which reduces boilerplate when defining them - Jobs no longer need to be registered with the runner - Rework of how `run_all_pending_jobs` works so errors loading the job actually bubble up - Configurable timeout for how long to wait for a job to begin running We aren't fully taking advantage of the timeout yet, I'd eventually like to try rebuilding the runner in-process a few times when an error happens, and then crash the process if it fails a few times in a row. This needs a bit more refactoring though (I'm not sure if we want to keep the `Repository` in memory like we are now, and whether we want to assume a full re-clone is needed on error). For now I've set it to ensure our jobs don't hang forever though. Warts in this version: - Rust thinks imports only used in the job body are unused. (https://github.com/sgrif/swirl/issues/6) - Worked around for now by moving the imports into the job body - `swirl::Job` has to be imported by calling code - Codegen assumes that `serde::Deserialize` and `serde::Serialize` are available. (https://github.com/sgrif/swirl/issues/9) Issues with this version: - The new impl of `run_all_pending_jobs` will return an error if the DB is in read only mode. If the DB is read only, we couldn't have enqueued jobs anyway, so the workaround is "fine", but we need some more robust APIs in swirl to fix this. This only affects us in tests. - https://github.com/sgrif/swirl/issues/8 --- Cargo.lock | 66 ++++++- Cargo.toml | 5 +- src/background_jobs.rs | 14 +- src/bin/background-worker.rs | 6 +- src/bin/monitor.rs | 3 - src/bin/on_call/mod.rs | 6 +- src/bin/render-readmes.rs | 2 +- src/bin/test-pagerduty.rs | 3 - src/controllers/krate/publish.rs | 28 +-- src/controllers/version/yank.rs | 12 +- src/git.rs | 181 ++++++++---------- src/lib.rs | 2 +- src/middleware/run_pending_background_jobs.rs | 20 +- src/tests/all.rs | 2 +- 14 files changed, 195 insertions(+), 155 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e63781c3803..a1730801f6f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -205,9 +205,8 @@ dependencies = [ "scheduled-thread-pool 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "semver 0.9.0 (git+https://github.com/steveklabnik/semver.git)", "serde 1.0.89 (registry+https://github.com/rust-lang/crates.io-index)", - "serde_derive 1.0.89 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.39 (registry+https://github.com/rust-lang/crates.io-index)", - "swirl 0.1.0 (git+https://github.com/sgrif/swirl.git?rev=95d3a35bc39a7274335cad6d7cab64acd5eb3904)", + "swirl 0.1.0 (git+https://github.com/sgrif/swirl.git?rev=de5d8bb)", "tar 0.4.21 (registry+https://github.com/rust-lang/crates.io-index)", "tempdir 0.3.7 (registry+https://github.com/rust-lang/crates.io-index)", "tokio-core 0.1.17 (registry+https://github.com/rust-lang/crates.io-index)", @@ -457,6 +456,15 @@ dependencies = [ "lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "ctor" +version = "0.1.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "quote 0.6.11 (registry+https://github.com/rust-lang/crates.io-index)", + "syn 0.15.29 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "curl" version = "0.4.12" @@ -846,6 +854,16 @@ dependencies = [ "typenum 1.10.0 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "ghost" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "proc-macro2 0.4.27 (registry+https://github.com/rust-lang/crates.io-index)", + "quote 0.6.11 (registry+https://github.com/rust-lang/crates.io-index)", + "syn 0.15.29 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "git2" version = "0.8.0" @@ -986,6 +1004,26 @@ name = "indexmap" version = "1.0.2" source = "registry+https://github.com/rust-lang/crates.io-index" +[[package]] +name = "inventory" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "ctor 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", + "ghost 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "inventory-impl 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", +] + +[[package]] +name = "inventory-impl" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "proc-macro2 0.4.27 (registry+https://github.com/rust-lang/crates.io-index)", + "quote 0.6.11 (registry+https://github.com/rust-lang/crates.io-index)", + "syn 0.15.29 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "iovec" version = "0.1.2" @@ -1973,6 +2011,9 @@ source = "registry+https://github.com/rust-lang/crates.io-index" name = "serde" version = "1.0.89" source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "serde_derive 1.0.89 (registry+https://github.com/rust-lang/crates.io-index)", +] [[package]] name = "serde_derive" @@ -2094,14 +2135,26 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "swirl" version = "0.1.0" -source = "git+https://github.com/sgrif/swirl.git?rev=95d3a35bc39a7274335cad6d7cab64acd5eb3904#95d3a35bc39a7274335cad6d7cab64acd5eb3904" +source = "git+https://github.com/sgrif/swirl.git?rev=de5d8bb#de5d8bbcfbf7878aa2c248c105938567b32ed8a8" dependencies = [ "diesel 1.4.1 (registry+https://github.com/rust-lang/crates.io-index)", + "inventory 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", "serde 1.0.89 (registry+https://github.com/rust-lang/crates.io-index)", "serde_json 1.0.39 (registry+https://github.com/rust-lang/crates.io-index)", + "swirl_proc_macro 0.1.0 (git+https://github.com/sgrif/swirl.git?rev=de5d8bb)", "threadpool 1.7.1 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "swirl_proc_macro" +version = "0.1.0" +source = "git+https://github.com/sgrif/swirl.git?rev=de5d8bb#de5d8bbcfbf7878aa2c248c105938567b32ed8a8" +dependencies = [ + "proc-macro2 0.4.27 (registry+https://github.com/rust-lang/crates.io-index)", + "quote 0.6.11 (registry+https://github.com/rust-lang/crates.io-index)", + "syn 0.15.29 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "syn" version = "0.11.11" @@ -2728,6 +2781,7 @@ dependencies = [ "checksum crossbeam-epoch 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)" = "04c9e3102cc2d69cd681412141b390abd55a362afc1540965dad0ad4d34280b4" "checksum crossbeam-queue 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "7c979cd6cfe72335896575c6b5688da489e420d36a27a0b9eb0c73db574b4a4b" "checksum crossbeam-utils 0.6.5 (registry+https://github.com/rust-lang/crates.io-index)" = "f8306fcef4a7b563b76b7dd949ca48f52bc1141aa067d2ea09565f3e2652aa5c" +"checksum ctor 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "9a43db2bba5cafdc6aa068c892a518e477ee0df3705e53ec70247a9ff93546d5" "checksum curl 0.4.12 (registry+https://github.com/rust-lang/crates.io-index)" = "aaf20bbe084f285f215eef2165feed70d6b75ba29cad24469badb853a4a287d0" "checksum curl-sys 0.4.16 (registry+https://github.com/rust-lang/crates.io-index)" = "ca79238a79fb294be6173b4057c95b22a718c94c4e38475d5faa82b8383f3502" "checksum dbghelp-sys 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "97590ba53bcb8ac28279161ca943a924d1fd4a8fb3fa63302591647c4fc5b850" @@ -2773,6 +2827,7 @@ dependencies = [ "checksum futures 0.1.25 (registry+https://github.com/rust-lang/crates.io-index)" = "49e7653e374fe0d0c12de4250f0bdb60680b8c80eed558c5c7538eec9c89e21b" "checksum futures-cpupool 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)" = "ab90cde24b3319636588d0c35fe03b1333857621051837ed769faefb4c2162e4" "checksum generic-array 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "ef25c5683767570c2bbd7deba372926a55eaae9982d7726ee2a1050239d45b9d" +"checksum ghost 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5297b71943dc9fea26a3241b178c140ee215798b7f79f7773fd61683e25bca74" "checksum git2 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "c7339329bfa14a00223244311560d11f8f489b453fb90092af97f267a6090ab0" "checksum h2 0.1.16 (registry+https://github.com/rust-lang/crates.io-index)" = "ddb2b25a33e231484694267af28fec74ac63b5ccf51ee2065a5e313b834d836e" "checksum hex 0.3.2 (registry+https://github.com/rust-lang/crates.io-index)" = "805026a5d0141ffc30abb3be3173848ad46a1b1664fe632428479619a3644d77" @@ -2786,6 +2841,8 @@ dependencies = [ "checksum hyper-tls 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "caaee4dea92794a9e697038bd401e264307d1f22c883dbcb6f6618ba0d3b3bd3" "checksum idna 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)" = "38f09e0f0b1fb55fdee1f17470ad800da77af5186a1a76c026b679358b7e844e" "checksum indexmap 1.0.2 (registry+https://github.com/rust-lang/crates.io-index)" = "7e81a7c05f79578dbc15793d8b619db9ba32b4577003ef3af1a91c416798c58d" +"checksum inventory 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "21df85981fe094480bc2267723d3dc0fd1ae0d1f136affc659b7398be615d922" +"checksum inventory-impl 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)" = "8a877ae8bce77402d5e9ed870730939e097aad827b2a932b361958fa9d6e75aa" "checksum iovec 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)" = "dbe6e417e7d0975db6512b90796e8ce223145ac4e33c377e4a42882a0e88bb08" "checksum itoa 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)" = "1306f3464951f30e30d12373d31c79fbd52d236e5e896fd92f96ec7babbbe60b" "checksum jemalloc-ctl 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)" = "4e93b0f37e7d735c6b610176d5b1bde8e1621ff3f6f7ac23cdfa4e7f7d0111b5" @@ -2914,7 +2971,8 @@ dependencies = [ "checksum string_cache_codegen 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)" = "1eea1eee654ef80933142157fdad9dd8bc43cf7c74e999e369263496f04ff4da" "checksum string_cache_shared 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b1884d1bc09741d466d9b14e6d37ac89d6909cbcac41dd9ae982d4d063bbedfc" "checksum strsim 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b4d15c810519a91cf877e7e36e63fe068815c678181439f2f29e2562147c3694" -"checksum swirl 0.1.0 (git+https://github.com/sgrif/swirl.git?rev=95d3a35bc39a7274335cad6d7cab64acd5eb3904)" = "" +"checksum swirl 0.1.0 (git+https://github.com/sgrif/swirl.git?rev=de5d8bb)" = "" +"checksum swirl_proc_macro 0.1.0 (git+https://github.com/sgrif/swirl.git?rev=de5d8bb)" = "" "checksum syn 0.11.11 (registry+https://github.com/rust-lang/crates.io-index)" = "d3b891b9015c88c576343b9b3e41c2c11a51c219ef067b264bd9c8aa9b441dad" "checksum syn 0.14.2 (registry+https://github.com/rust-lang/crates.io-index)" = "c67da57e61ebc7b7b6fff56bb34440ca3a83db037320b0507af4c10368deda7d" "checksum syn 0.15.29 (registry+https://github.com/rust-lang/crates.io-index)" = "1825685f977249735d510a242a6727b46efe914bb67e38d30c071b1b72b1d5c2" diff --git a/Cargo.toml b/Cargo.toml index e96d76a9d0b..baa549a1bb6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,10 +48,9 @@ dotenv = "0.11.0" toml = "0.4" diesel = { version = "1.4.0", features = ["postgres", "serde_json", "chrono", "r2d2"] } diesel_full_text_search = "1.0.0" -swirl = { git = "https://github.com/sgrif/swirl.git", rev = "95d3a35bc39a7274335cad6d7cab64acd5eb3904" } +swirl = { git = "https://github.com/sgrif/swirl.git", rev = "de5d8bb" } serde_json = "1.0.0" -serde_derive = "1.0.0" -serde = "1.0.0" +serde = { version = "1.0.0", features = ["derive"] } chrono = { version = "0.4.0", features = ["serde"] } comrak = { version = "0.4.0", default-features = false } ammonia = "2.0.0" diff --git a/src/background_jobs.rs b/src/background_jobs.rs index ec3c8b57d0b..d3ede8f2c11 100644 --- a/src/background_jobs.rs +++ b/src/background_jobs.rs @@ -1,24 +1,22 @@ use std::panic::AssertUnwindSafe; use std::sync::{Mutex, MutexGuard}; -use swirl::{Builder, Runner}; use crate::db::{DieselPool, DieselPooledConn}; -use crate::git::{AddCrate, Repository, Yank}; +use crate::git::Repository; use crate::util::errors::{CargoErrToStdErr, CargoResult}; -impl<'a> swirl::DieselPool<'a> for DieselPool { +impl<'a> swirl::db::BorrowedConnection<'a> for DieselPool { type Connection = DieselPooledConn<'a>; +} + +impl swirl::db::DieselPool for DieselPool { type Error = CargoErrToStdErr; - fn get(&'a self) -> Result { + fn get(&self) -> Result, Self::Error> { self.get().map_err(CargoErrToStdErr) } } -pub fn job_runner(config: Builder) -> Runner { - config.register::().register::().build() -} - #[allow(missing_debug_implementations)] pub struct Environment { index: Mutex, diff --git a/src/bin/background-worker.rs b/src/bin/background-worker.rs index acd50ddcf64..99b93481a19 100644 --- a/src/bin/background-worker.rs +++ b/src/bin/background-worker.rs @@ -37,8 +37,10 @@ fn main() { let environment = Environment::new(repository, credentials, db_pool.clone()); - let builder = swirl::Runner::builder(db_pool, environment).thread_count(1); - let runner = job_runner(builder); + let runner = swirl::Runner::builder(db_pool, environment) + .thread_count(1) + .job_start_timeout(Duration::from_secs(10)) + .build(); println!("Runner booted, running jobs"); diff --git a/src/bin/monitor.rs b/src/bin/monitor.rs index 5fc93f47f5c..b6844560d8e 100644 --- a/src/bin/monitor.rs +++ b/src/bin/monitor.rs @@ -6,9 +6,6 @@ #![deny(warnings)] -#[macro_use] -extern crate serde_derive; - mod on_call; use cargo_registry::{db, util::CargoResult}; diff --git a/src/bin/on_call/mod.rs b/src/bin/on_call/mod.rs index e76add72258..4e99d83b163 100644 --- a/src/bin/on_call/mod.rs +++ b/src/bin/on_call/mod.rs @@ -2,7 +2,7 @@ use cargo_registry::util::{internal, CargoResult}; use reqwest::{header, StatusCode as Status}; -#[derive(Serialize, Debug)] +#[derive(serde::Serialize, Debug)] #[serde(rename_all = "snake_case", tag = "event_type")] pub enum Event { Trigger { @@ -54,14 +54,14 @@ impl Event { } } -#[derive(Serialize, Debug)] +#[derive(serde::Serialize, Debug)] struct FullEvent { service_key: String, #[serde(flatten)] event: Event, } -#[derive(Deserialize, Debug)] +#[derive(serde::Deserialize, Debug)] struct InvalidEvent { message: String, errors: Vec, diff --git a/src/bin/render-readmes.rs b/src/bin/render-readmes.rs index ea636401a33..6d8e3b6fd09 100644 --- a/src/bin/render-readmes.rs +++ b/src/bin/render-readmes.rs @@ -6,7 +6,7 @@ #![deny(warnings)] #[macro_use] -extern crate serde_derive; +extern crate serde; use cargo_registry::{ db, diff --git a/src/bin/test-pagerduty.rs b/src/bin/test-pagerduty.rs index e00c4b0e980..a2baffc342c 100644 --- a/src/bin/test-pagerduty.rs +++ b/src/bin/test-pagerduty.rs @@ -7,9 +7,6 @@ #![deny(warnings)] -#[macro_use] -extern crate serde_derive; - mod on_call; use std::env::args; diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index c88cddcbbb6..81c7cc586b5 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -1,18 +1,17 @@ //! Functionality related to publishing a new crate or version of a crate. +use hex::ToHex; use std::collections::HashMap; use std::sync::Arc; - -use hex::ToHex; - -use crate::git; -use crate::render; -use crate::util::{internal, ChainError, Maximums}; -use crate::util::{read_fill, read_le_u32}; +use swirl::Job; use crate::controllers::prelude::*; +use crate::git; use crate::models::dependency; use crate::models::{Badge, Category, Keyword, NewCrate, NewVersion, Rights, User}; +use crate::render; +use crate::util::{internal, CargoError, ChainError, Maximums}; +use crate::util::{read_fill, read_le_u32}; use crate::views::{EncodableCrateUpload, GoodCrate, PublishWarnings}; /// Handles the `PUT /crates/new` route. @@ -196,12 +195,15 @@ pub fn publish(req: &mut dyn Request) -> CargoResult { yanked: Some(false), links, }; - git::add_crate(&conn, git_crate).chain_error(|| { - internal(&format_args!( - "could not add crate `{}` to the git repo", - name - )) - })?; + git::add_crate(git_crate) + .enqueue(&conn) + .map_err(|e| CargoError::from_std_error(e)) + .chain_error(|| { + internal(&format_args!( + "could not add crate `{}` to the git repo", + name + )) + })?; // Now that we've come this far, we're committed! crate_bomb.path = None; diff --git a/src/controllers/version/yank.rs b/src/controllers/version/yank.rs index 43136e2eb22..dd1105d3501 100644 --- a/src/controllers/version/yank.rs +++ b/src/controllers/version/yank.rs @@ -1,12 +1,12 @@ //! Endpoints for yanking and unyanking specific versions of crates -use crate::controllers::prelude::*; +use swirl::Job; +use super::version_and_crate; +use crate::controllers::prelude::*; use crate::git; - use crate::models::Rights; - -use super::version_and_crate; +use crate::util::CargoError; /// Handles the `DELETE /crates/:crate_id/:version/yank` route. /// This does not delete a crate version, it makes the crate @@ -36,7 +36,9 @@ fn modify_yank(req: &mut dyn Request, yanked: bool) -> CargoResult { return Err(human("must already be an owner to yank or unyank")); } - git::yank(&conn, krate.name, version, yanked)?; + git::yank(krate.name, version, yanked) + .enqueue(&conn) + .map_err(|e| CargoError::from_std_error(e))?; #[derive(Serialize)] struct R { diff --git a/src/git.rs b/src/git.rs index 07f454c85a9..fcb7e008325 100644 --- a/src/git.rs +++ b/src/git.rs @@ -1,18 +1,16 @@ #![allow(missing_debug_implementations)] -use diesel::prelude::*; use std::collections::HashMap; use std::fs::{self, OpenOptions}; -use std::io::prelude::*; use std::path::{Path, PathBuf}; -use swirl::{errors::PerformError, Job}; +use swirl::errors::PerformError; use tempdir::TempDir; use url::Url; use crate::background_jobs::Environment; use crate::models::{DependencyKind, Version}; use crate::schema::versions; -use crate::util::errors::{std_error_no_send, CargoError, CargoResult}; +use crate::util::errors::{std_error_no_send, CargoResult}; #[derive(Serialize, Deserialize, Debug)] pub struct Crate { @@ -131,115 +129,88 @@ impl Repository { } } -#[derive(Deserialize, Serialize)] -pub struct AddCrate { - krate: Crate, -} - -impl Job for AddCrate { - type Environment = Environment; - const JOB_TYPE: &'static str = "add_crate"; +#[swirl::background_job] +pub fn add_crate(env: &Environment, krate: Crate) -> Result<(), PerformError> { + use std::io::prelude::*; - fn perform(self, env: &Self::Environment) -> Result<(), PerformError> { - let repo = env.lock_index().map_err(std_error_no_send)?; - let dst = repo.index_file(&self.krate.name); + let repo = env.lock_index().map_err(std_error_no_send)?; + let dst = repo.index_file(&krate.name); - // Add the crate to its relevant file - fs::create_dir_all(dst.parent().unwrap())?; - let mut file = OpenOptions::new().append(true).create(true).open(&dst)?; - serde_json::to_writer(&mut file, &self.krate)?; - file.write_all(b"\n")?; - - repo.commit_and_push( - &format!("Updating crate `{}#{}`", self.krate.name, self.krate.vers), - &repo.relative_index_file(&self.krate.name), - env.credentials(), - ) - } -} + // Add the crate to its relevant file + fs::create_dir_all(dst.parent().unwrap())?; + let mut file = OpenOptions::new().append(true).create(true).open(&dst)?; + serde_json::to_writer(&mut file, &krate)?; + file.write_all(b"\n")?; -pub fn add_crate(conn: &PgConnection, krate: Crate) -> CargoResult<()> { - AddCrate { krate } - .enqueue(conn) - .map_err(|e| CargoError::from_std_error(e)) + repo.commit_and_push( + &format!("Updating crate `{}#{}`", krate.name, krate.vers), + &repo.relative_index_file(&krate.name), + env.credentials(), + ) } -#[derive(Serialize, Deserialize)] -pub struct Yank { +/// Yanks or unyanks a crate version. This requires finding the index +/// file, deserlialise the crate from JSON, change the yank boolean to +/// `true` or `false`, write all the lines back out, and commit and +/// push the changes. +#[swirl::background_job] +pub fn yank( + env: &Environment, krate: String, version: Version, yanked: bool, -} +) -> Result<(), PerformError> { + use diesel::prelude::*; -impl Job for Yank { - type Environment = Environment; - const JOB_TYPE: &'static str = "yank"; - - fn perform(self, env: &Self::Environment) -> Result<(), PerformError> { - let repo = env.lock_index().map_err(std_error_no_send)?; - let dst = repo.index_file(&self.krate); - - let conn = env.connection().map_err(std_error_no_send)?; - - conn.transaction(|| { - let yanked_in_db = versions::table - .find(self.version.id) - .select(versions::yanked) - .for_update() - .first::(&*conn)?; - - if yanked_in_db == self.yanked { - // The crate is alread in the state requested, nothing to do - return Ok(()); - } - - let prev = fs::read_to_string(&dst)?; - let version = self.version.num.to_string(); - let new = prev - .lines() - .map(|line| { - let mut git_crate = serde_json::from_str::(line) - .map_err(|_| format!("couldn't decode: `{}`", line))?; - if git_crate.name != self.krate || git_crate.vers != version { - return Ok(line.to_string()); - } - git_crate.yanked = Some(self.yanked); - Ok(serde_json::to_string(&git_crate)?) - }) - .collect::, PerformError>>(); - let new = new?.join("\n") + "\n"; - fs::write(&dst, new.as_bytes())?; - - repo.commit_and_push( - &format!( - "{} crate `{}#{}`", - if self.yanked { "Yanking" } else { "Unyanking" }, - self.krate, - self.version.num - ), - &repo.relative_index_file(&self.krate), - env.credentials(), - )?; - - diesel::update(&self.version) - .set(versions::yanked.eq(self.yanked)) - .execute(&*conn)?; - - Ok(()) - }) - } -} + let repo = env.lock_index().map_err(std_error_no_send)?; + let dst = repo.index_file(&krate); -/// Yanks or unyanks a crate version. This requires finding the index -/// file, deserlialise the crate from JSON, change the yank boolean to -/// `true` or `false`, write all the lines back out, and commit and -/// push the changes. -pub fn yank(conn: &PgConnection, krate: String, version: Version, yanked: bool) -> CargoResult<()> { - Yank { - krate, - version, - yanked, - } - .enqueue(conn) - .map_err(|e| CargoError::from_std_error(e)) + let conn = env.connection().map_err(std_error_no_send)?; + + conn.transaction(|| { + let yanked_in_db = versions::table + .find(version.id) + .select(versions::yanked) + .for_update() + .first::(&*conn)?; + + if yanked_in_db == yanked { + // The crate is alread in the state requested, nothing to do + return Ok(()); + } + + let prev = fs::read_to_string(&dst)?; + let version_num = version.num.to_string(); + let new = prev + .lines() + .map(|line| { + let mut git_crate = serde_json::from_str::(line) + .map_err(|_| format!("couldn't decode: `{}`", line))?; + if git_crate.name != krate || git_crate.vers != version_num { + return Ok(line.to_string()); + } + git_crate.yanked = Some(yanked); + Ok(serde_json::to_string(&git_crate)?) + }) + .collect::, PerformError>>(); + let new = new?.join("\n") + "\n"; + fs::write(&dst, new.as_bytes())?; + + repo.commit_and_push( + &format!( + "{} crate `{}#{}`", + if yanked { "Yanking" } else { "Unyanking" }, + krate, + version.num + ), + &repo.relative_index_file(&krate), + env.credentials(), + )?; + + diesel::update(&version) + .set(versions::yanked.eq(yanked)) + .execute(&*conn)?; + + Ok(()) + }) } diff --git a/src/lib.rs b/src/lib.rs index 58b9bc09874..a720e9c18a6 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -15,7 +15,7 @@ extern crate diesel; #[macro_use] extern crate log; #[macro_use] -extern crate serde_derive; +extern crate serde; #[macro_use] extern crate serde_json; diff --git a/src/middleware/run_pending_background_jobs.rs b/src/middleware/run_pending_background_jobs.rs index 5023aee7a7a..dbea033b7d6 100644 --- a/src/middleware/run_pending_background_jobs.rs +++ b/src/middleware/run_pending_background_jobs.rs @@ -1,3 +1,4 @@ +use std::time::Duration; use swirl::Runner; use super::app::RequestApp; @@ -18,14 +19,27 @@ impl Middleware for RunPendingBackgroundJobs { } let app = req.app(); + let connection_pool = app.diesel_database.clone(); let repo = Repository::open(&app.config.index_location).expect("Could not clone index"); let environment = Environment::new(repo, None, app.diesel_database.clone()); - let config = Runner::builder(connection_pool, environment); - let runner = job_runner(config); + let runner = Runner::builder(connection_pool, environment) + // We only have 1 connection in tests, so trying to run more than + // 1 job concurrently will just block + .thread_count(1) + .job_start_timeout(Duration::from_secs(1)) + .build(); + + // FIXME: https://github.com/sgrif/swirl/issues/8 + if let Err(e) = runner.run_all_pending_jobs() { + if e.to_string().ends_with("read-only transaction") { + return res; + } else { + panic!("Could not run jobs: {}", e); + } + } - runner.run_all_pending_jobs().expect("Could not run jobs"); runner .assert_no_failed_jobs() .expect("Could not determine if jobs failed"); diff --git a/src/tests/all.rs b/src/tests/all.rs index 35dea7a5aff..0631ad513db 100644 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -5,7 +5,7 @@ extern crate diesel; #[macro_use] extern crate lazy_static; #[macro_use] -extern crate serde_derive; +extern crate serde; #[macro_use] extern crate serde_json;