From 1ea6ae19cf0b2a56fd773faec46f6b61db136010 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Thu, 13 Oct 2022 21:30:50 +0200 Subject: [PATCH 1/7] add simple "deployment pending" marker 20 minutes after a build --- src/cdn.rs | 24 ++++++++++++++++++++++++ src/web/releases.rs | 4 ++++ templates/core/home.html | 13 ++++++++++++- templates/releases/releases.html | 13 ++++++++++++- templates/style/style.scss | 8 ++++++++ 5 files changed, 60 insertions(+), 2 deletions(-) diff --git a/src/cdn.rs b/src/cdn.rs index 25235dd58..6cf20319d 100644 --- a/src/cdn.rs +++ b/src/cdn.rs @@ -4,6 +4,7 @@ use aws_sdk_cloudfront::{ model::{InvalidationBatch, Paths}, Client, RetryConfig, }; +use chrono::{DateTime, Utc}; use std::sync::{Arc, Mutex}; use strum::EnumString; use tokio::runtime::Runtime; @@ -133,15 +134,38 @@ pub(crate) fn invalidate_crate(config: &Config, cdn: &CdnBackend, name: &str) -> Ok(()) } +/// Return if we count the deploy as pending based on the build-time. +/// CloudFront invalidations can take up to 15 minutes. Until we have +/// live queries of the invalidation status we just assume it's fine +/// latest 20 minutes after the build. +/// TODO: should be replaced be keeping track or querying the active invalidation from CloudFront +pub(crate) fn crate_invalidation_pending(build_time: &DateTime) -> bool { + Utc::now().signed_duration_since(*build_time) <= chrono::Duration::minutes(20) +} + #[cfg(test)] mod tests { use super::*; use crate::test::wrapper; + use chrono::Duration; + use test_case::test_case; use aws_sdk_cloudfront::{Client, Config, Credentials, Region}; use aws_smithy_client::{erase::DynConnector, test_connection::TestConnection}; use aws_smithy_http::body::SdkBody; + #[test_case(10, true)] + #[test_case(19, true)] + #[test_case(21, false)] + #[test_case(9999, false)] + fn get_invalidation_pending(minutes: i64, expected: bool) { + let now = Utc::now(); + assert_eq!( + crate_invalidation_pending(&(now - Duration::minutes(minutes))), + expected + ); + } + #[test] fn create_cloudfront() { wrapper(|env| { diff --git a/src/web/releases.rs b/src/web/releases.rs index db7463645..4ad57eca0 100644 --- a/src/web/releases.rs +++ b/src/web/releases.rs @@ -2,6 +2,7 @@ use crate::{ build_queue::QueuedCrate, + cdn::crate_invalidation_pending, db::{Pool, PoolClient}, impl_webpage, utils::report_error, @@ -38,6 +39,7 @@ pub struct Release { description: Option, target_name: Option, rustdoc_status: bool, + deployment_pending: bool, pub(crate) build_time: DateTime, stars: i32, } @@ -109,6 +111,7 @@ pub(crate) fn get_releases( target_name: row.get(3), rustdoc_status: row.get(4), build_time: row.get(5), + deployment_pending: crate_invalidation_pending(&row.get(5)), stars: row.get::<_, Option>(6).unwrap_or(0), }) .collect() @@ -222,6 +225,7 @@ fn get_search_results( version: row.get("version"), description: row.get("description"), build_time: row.get("build_time"), + deployment_pending: crate_invalidation_pending(&row.get("build_time")), target_name: row.get("target_name"), rustdoc_status: row.get("rustdoc_status"), stars: stars.unwrap_or(0), diff --git a/templates/core/home.html b/templates/core/home.html index 1c4593ae2..f13ca483d 100644 --- a/templates/core/home.html +++ b/templates/core/home.html @@ -47,7 +47,7 @@

{{ "cubes" | fas(fw=true) }} Docs.rs

{{ release.name }}-{{ release.version }}
-
+
{{ release.description }}
@@ -62,6 +62,17 @@

{{ "cubes" | fas(fw=true) }} Docs.rs

{{ release.build_time | timeformat(relative=true) }}
{%- endif -%} + {% if release.deployment_pending -%} +
+ {{ "spinner" | fas }} +
+ {%- else -%} +
+ {{ "check" | fas }} +
+ {%- endif -%} diff --git a/templates/releases/releases.html b/templates/releases/releases.html index c83c3bd9f..19b921e8a 100644 --- a/templates/releases/releases.html +++ b/templates/releases/releases.html @@ -34,7 +34,7 @@ {{ release.name }}-{{ release.version }} -
+
{{ release.description }}
@@ -50,6 +50,17 @@ {{ release.build_time | timeformat(relative=true) }}
{%- endif %} + {% if release.deployment_pending -%} +
+ {{ "spinner" | fas }} +
+ {%- else -%} +
+ {{ "check" | fas }} +
+ {%- endif -%} diff --git a/templates/style/style.scss b/templates/style/style.scss index 0063f60ec..cb7ea088f 100644 --- a/templates/style/style.scss +++ b/templates/style/style.scss @@ -364,6 +364,14 @@ div.recent-releases-container { } } + .deploymentstatus { + font-weight: normal; + + @media #{$media-sm} { + text-align: right; + } + } + div.pagination { text-align: center; margin: 1em; From 22ca2f56eb1b4e2756e6be49b5e4e2a790d3fab4 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Sat, 15 Oct 2022 08:31:24 +0200 Subject: [PATCH 2/7] update tooltip for "deployment pending" icon --- templates/core/home.html | 2 +- templates/releases/releases.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/templates/core/home.html b/templates/core/home.html index f13ca483d..0b7fd912f 100644 --- a/templates/core/home.html +++ b/templates/core/home.html @@ -64,7 +64,7 @@

{{ "cubes" | fas(fw=true) }} Docs.rs

{%- endif -%} {% if release.deployment_pending -%}
+ title="deployment pending, it may take up to 20 minutes for this version to be visible to everybody"> {{ "spinner" | fas }}
{%- else -%} diff --git a/templates/releases/releases.html b/templates/releases/releases.html index 19b921e8a..183d7d487 100644 --- a/templates/releases/releases.html +++ b/templates/releases/releases.html @@ -52,7 +52,7 @@ {%- endif %} {% if release.deployment_pending -%}
+ title="deployment pending, it may take up to 20 minutes for this version to be visible to everybody"> {{ "spinner" | fas }}
{%- else -%} From 5e26abfd8621960031c60e449d15e109f9fe6ff7 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Sat, 15 Oct 2022 16:43:43 +0200 Subject: [PATCH 3/7] only show deployment status in detailed release-lists, remove from homepage --- templates/core/home.html | 13 +------------ templates/releases/releases.html | 21 ++++++++++++++------- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/templates/core/home.html b/templates/core/home.html index 0b7fd912f..1c4593ae2 100644 --- a/templates/core/home.html +++ b/templates/core/home.html @@ -47,7 +47,7 @@

{{ "cubes" | fas(fw=true) }} Docs.rs

{{ release.name }}-{{ release.version }}
-
+
{{ release.description }}
@@ -62,17 +62,6 @@

{{ "cubes" | fas(fw=true) }} Docs.rs

{{ release.build_time | timeformat(relative=true) }}
{%- endif -%} - {% if release.deployment_pending -%} -
- {{ "spinner" | fas }} -
- {%- else -%} -
- {{ "check" | fas }} -
- {%- endif -%} diff --git a/templates/releases/releases.html b/templates/releases/releases.html index 183d7d487..0dc8923bd 100644 --- a/templates/releases/releases.html +++ b/templates/releases/releases.html @@ -50,15 +50,22 @@ {{ release.build_time | timeformat(relative=true) }} {%- endif %} - {% if release.deployment_pending -%} -
- {{ "spinner" | fas }} -
+ {%- if release.rustdoc_status -%} + {%- if release.deployment_pending -%} +
+ {{ "spinner" | fas }} +
+ {%- else -%} +
+ {{ "check" | fas }} +
+ {%- endif -%} {%- else -%}
- {{ "check" | fas }} + title="build error"> + {{ "triangle-exclamation" | fas(fw=true) }}
{%- endif -%} From 2280863409cd42e0f3721aa6f983877298fbce25 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Sat, 15 Oct 2022 17:44:21 +0200 Subject: [PATCH 4/7] update deployment tooltip --- templates/releases/releases.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/templates/releases/releases.html b/templates/releases/releases.html index 0dc8923bd..f610e075b 100644 --- a/templates/releases/releases.html +++ b/templates/releases/releases.html @@ -53,7 +53,7 @@ {%- if release.rustdoc_status -%} {%- if release.deployment_pending -%}
+ title="deployment pending, it may take up to 20 minutes for this version to be visible to everybody and on all pages. Especially `/latest/` URLs might be affected."> {{ "spinner" | fas }}
{%- else -%} From 2d94fa64cc24eebe111d8465d75fb8d69be27389 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Sat, 15 Oct 2022 19:09:42 +0200 Subject: [PATCH 5/7] don't show un-deployed releases on homepage --- src/web/releases.rs | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/src/web/releases.rs b/src/web/releases.rs index 4ad57eca0..4d6e2b309 100644 --- a/src/web/releases.rs +++ b/src/web/releases.rs @@ -64,6 +64,7 @@ pub(crate) fn get_releases( limit: i64, order: Order, latest_only: bool, + deployed_only: bool, ) -> Vec { let offset = (page - 1) * limit; @@ -104,6 +105,7 @@ pub(crate) fn get_releases( conn.query(query.as_str(), &[&limit, &offset, &filter_failed]) .unwrap() .into_iter() + .filter(|row| !deployed_only || !crate_invalidation_pending(&row.get(5))) .map(|row| Release { name: row.get(0), version: row.get(1), @@ -260,7 +262,14 @@ impl_webpage! { pub fn home_page(req: &mut Request) -> IronResult { let mut conn = extension!(req, Pool).get()?; - let recent_releases = get_releases(&mut conn, 1, RELEASES_IN_HOME, Order::ReleaseTime, true); + let recent_releases = get_releases( + &mut conn, + 1, + RELEASES_IN_HOME, + Order::ReleaseTime, + true, + true, + ); HomePage { recent_releases }.into_response(req) } @@ -277,7 +286,14 @@ impl_webpage! { pub fn releases_feed_handler(req: &mut Request) -> IronResult { let mut conn = extension!(req, Pool).get()?; - let recent_releases = get_releases(&mut conn, 1, RELEASES_IN_FEED, Order::ReleaseTime, true); + let recent_releases = get_releases( + &mut conn, + 1, + RELEASES_IN_FEED, + Order::ReleaseTime, + true, + false, + ); ReleaseFeed { recent_releases }.into_response(req) } @@ -340,6 +356,7 @@ fn releases_handler(req: &mut Request, release_type: ReleaseType) -> IronResult< RELEASES_IN_RELEASES, release_order, latest_only, + false, ) }; @@ -710,6 +727,21 @@ mod tests { use std::collections::HashSet; use test_case::test_case; + #[test] + fn get_releases_only_deployed() { + wrapper(|env| { + let db = env.db(); + + env.fake_release().name("foo").version("1.0.0").create()?; + + assert!( + !get_releases(&mut db.conn(), 1, 10, Order::ReleaseTime, true, false).is_empty() + ); + assert!(get_releases(&mut db.conn(), 1, 10, Order::ReleaseTime, true, true).is_empty()); + Ok(()) + }) + } + #[test] fn get_releases_by_stars() { wrapper(|env| { @@ -728,7 +760,7 @@ mod tests { // release without stars will not be shown env.fake_release().name("baz").version("1.0.0").create()?; - let releases = get_releases(&mut db.conn(), 1, 10, Order::GithubStars, true); + let releases = get_releases(&mut db.conn(), 1, 10, Order::GithubStars, true, false); assert_eq!( vec![ "bar", // 20 stars @@ -739,7 +771,6 @@ mod tests { .map(|release| release.name.as_str()) .collect::>(), ); - Ok(()) }) } From e3ba37aa22c3ee0451490746dbf9667e0154c947 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Sat, 15 Oct 2022 19:49:55 +0200 Subject: [PATCH 6/7] fix & add tests for get-releases / deployed_only --- src/test/fakes.rs | 20 +++++++++++++++ src/web/releases.rs | 59 +++++++++++++++++++++++++++++++++++++++------ 2 files changed, 72 insertions(+), 7 deletions(-) diff --git a/src/test/fakes.rs b/src/test/fakes.rs index dc5252626..927bf4a23 100644 --- a/src/test/fakes.rs +++ b/src/test/fakes.rs @@ -38,6 +38,7 @@ pub(crate) struct FakeRelease<'a> { pub(crate) struct FakeBuild { s3_build_log: Option, db_build_log: Option, + build_time: Option>, result: BuildResult, } @@ -108,6 +109,11 @@ impl<'a> FakeRelease<'a> { self } + pub(crate) fn has_docs(mut self, new: bool) -> Self { + self.has_docs = new; + self + } + pub(crate) fn name(mut self, new: &str) -> Self { self.package.name = new.into(); self.package.id = format!("{}-id", new); @@ -458,6 +464,12 @@ impl FakeGithubStats { } impl FakeBuild { + pub(crate) fn build_time(self, build_time: impl Into>) -> Self { + Self { + build_time: Some(build_time.into()), + ..self + } + } pub(crate) fn rustc_version(self, rustc_version: impl Into) -> Self { Self { result: BuildResult { @@ -525,6 +537,13 @@ impl FakeBuild { )?; } + if let Some(build_time) = self.build_time.as_ref() { + conn.query( + "UPDATE builds SET build_time = $2 WHERE id = $1", + &[&build_id, &build_time], + )?; + } + if let Some(s3_build_log) = self.s3_build_log.as_deref() { let path = format!("build-logs/{}/{}.txt", build_id, default_target); storage.store_one(path, s3_build_log)?; @@ -539,6 +558,7 @@ impl Default for FakeBuild { Self { s3_build_log: Some("It works!".into()), db_build_log: None, + build_time: None, result: BuildResult { rustc_version: "rustc 2.0.0-nightly (000000000 1970-01-01)".into(), docsrs_version: "docs.rs 1.0.0 (000000000 1970-01-01)".into(), diff --git a/src/web/releases.rs b/src/web/releases.rs index 4d6e2b309..35060c296 100644 --- a/src/web/releases.rs +++ b/src/web/releases.rs @@ -716,7 +716,8 @@ mod tests { use super::*; use crate::index::api::CrateOwner; use crate::test::{ - assert_redirect, assert_redirect_unchecked, assert_success, wrapper, TestFrontend, + assert_redirect, assert_redirect_unchecked, assert_success, wrapper, FakeBuild, + TestFrontend, }; use anyhow::Error; use chrono::{Duration, TimeZone}; @@ -732,12 +733,36 @@ mod tests { wrapper(|env| { let db = env.db(); - env.fake_release().name("foo").version("1.0.0").create()?; + let now = Utc::now(); - assert!( - !get_releases(&mut db.conn(), 1, 10, Order::ReleaseTime, true, false).is_empty() + env.fake_release() + .name("recent") + .version("1.0.0") + .builds(vec![FakeBuild::default().build_time(now)]) + .create()?; + + env.fake_release() + .name("old") + .version("1.0.0") + .builds(vec![ + FakeBuild::default().build_time(now - Duration::minutes(21)) + ]) + .create()?; + + assert_eq!( + get_releases(&mut db.conn(), 1, 10, Order::ReleaseTime, true, false) + .iter() + .map(|r| r.name.clone()) + .collect::>(), + vec!["recent", "old"], + ); + assert_eq!( + get_releases(&mut db.conn(), 1, 10, Order::ReleaseTime, true, true) + .iter() + .map(|r| r.name.clone()) + .collect::>(), + vec!["old"], ); - assert!(get_releases(&mut db.conn(), 1, 10, Order::ReleaseTime, true, true).is_empty()); Ok(()) }) } @@ -1255,25 +1280,44 @@ mod tests { .version("0.1.0") .github_stats("some/repo", 33, 22, 11) .release_time(Utc.ymd(2020, 4, 16).and_hms(4, 33, 50)) + .builds(vec![ + FakeBuild::default().build_time(Utc.ymd(2020, 4, 16).and_hms(4, 33, 50)) + ]) .create()?; env.fake_release() .name("crate_that_succeeded_with_github") .version("0.2.0-rc") .github_stats("some/repo", 33, 22, 11) .release_time(Utc.ymd(2020, 4, 16).and_hms(8, 33, 50)) - .build_result_failed() + .has_docs(false) + .builds(vec![FakeBuild::default() + .build_time(Utc.ymd(2020, 4, 16).and_hms(8, 33, 50)) + .successful(false)]) .create()?; env.fake_release() .name("crate_that_succeeded_with_github") .github_stats("some/repo", 33, 22, 11) .release_time(Utc.ymd(2020, 5, 16).and_hms(4, 33, 50)) + .builds(vec![ + FakeBuild::default().build_time(Utc.ymd(2020, 5, 16).and_hms(4, 33, 50)) + ]) .version("0.2.0") .create()?; env.fake_release() .name("crate_that_failed") .version("0.1.0") .release_time(Utc.ymd(2020, 6, 16).and_hms(4, 33, 50)) - .build_result_failed() + .has_docs(false) + .builds(vec![FakeBuild::default() + .build_time(Utc.ymd(2020, 6, 16).and_hms(4, 33, 50)) + .successful(false)]) + .create()?; + let now = Utc::now(); + env.fake_release() + .name("crate_that_was_just_deployed_and_will_not_be_shown") + .version("0.1.0") + .release_time(now) + .builds(vec![FakeBuild::default().build_time(now)]) .create()?; // make sure that crates get at most one release shown, so they don't crowd the homepage @@ -1289,6 +1333,7 @@ mod tests { assert_eq!( get_release_links("/releases", env.frontend())?, [ + "/crate_that_was_just_deployed_and_will_not_be_shown/0.1.0/crate_that_was_just_deployed_and_will_not_be_shown/", "/crate/crate_that_failed/0.1.0", "/crate_that_succeeded_with_github/0.2.0/crate_that_succeeded_with_github/", "/crate/crate_that_succeeded_with_github/0.2.0-rc", From 9d79025b3018c5cbc1b9e8cd162da625457ffec6 Mon Sep 17 00:00:00 2001 From: Denis Cornehl Date: Tue, 18 Oct 2022 15:28:41 +0200 Subject: [PATCH 7/7] release-lists: don't show build failure for binary crates --- src/web/releases.rs | 14 ++++++++++++-- templates/releases/releases.html | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/web/releases.rs b/src/web/releases.rs index 35060c296..90e3c02c9 100644 --- a/src/web/releases.rs +++ b/src/web/releases.rs @@ -39,6 +39,8 @@ pub struct Release { description: Option, target_name: Option, rustdoc_status: bool, + build_status: bool, + is_library: bool, deployment_pending: bool, pub(crate) build_time: DateTime, stars: i32, @@ -83,7 +85,9 @@ pub(crate) fn get_releases( releases.target_name, releases.rustdoc_status, builds.build_time, - repositories.stars + repositories.stars, + releases.build_status, + releases.is_library FROM crates {1} INNER JOIN builds ON releases.id = builds.rid @@ -115,6 +119,8 @@ pub(crate) fn get_releases( build_time: row.get(5), deployment_pending: crate_invalidation_pending(&row.get(5)), stars: row.get::<_, Option>(6).unwrap_or(0), + build_status: row.get(7), + is_library: row.get(8), }) .collect() } @@ -206,7 +212,9 @@ fn get_search_results( builds.build_time, releases.target_name, releases.rustdoc_status, - repositories.stars + repositories.stars, + releases.build_status, + releases.is_library FROM crates INNER JOIN releases ON crates.latest_version_id = releases.id @@ -231,6 +239,8 @@ fn get_search_results( target_name: row.get("target_name"), rustdoc_status: row.get("rustdoc_status"), stars: stars.unwrap_or(0), + build_status: row.get("build_status"), + is_library: row.get("is_library"), }, ) }) diff --git a/templates/releases/releases.html b/templates/releases/releases.html index f610e075b..9f97fe1d2 100644 --- a/templates/releases/releases.html +++ b/templates/releases/releases.html @@ -62,7 +62,7 @@ {{ "check" | fas }} {%- endif -%} - {%- else -%} + {%- elif release.is_library and not release.build_status -%}
{{ "triangle-exclamation" | fas(fw=true) }}