Skip to content

add simple "deployment pending" marker 20 minutes after a build #1880

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

Closed
wants to merge 7 commits into from
Closed
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
24 changes: 24 additions & 0 deletions src/cdn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Utc>) -> 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| {
Expand Down
20 changes: 20 additions & 0 deletions src/test/fakes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub(crate) struct FakeRelease<'a> {
pub(crate) struct FakeBuild {
s3_build_log: Option<String>,
db_build_log: Option<String>,
build_time: Option<DateTime<Utc>>,
result: BuildResult,
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -458,6 +464,12 @@ impl FakeGithubStats {
}

impl FakeBuild {
pub(crate) fn build_time(self, build_time: impl Into<DateTime<Utc>>) -> Self {
Self {
build_time: Some(build_time.into()),
..self
}
}
pub(crate) fn rustc_version(self, rustc_version: impl Into<String>) -> Self {
Self {
result: BuildResult {
Expand Down Expand Up @@ -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)?;
Expand All @@ -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(),
Expand Down
108 changes: 99 additions & 9 deletions src/web/releases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

use crate::{
build_queue::QueuedCrate,
cdn::crate_invalidation_pending,
db::{Pool, PoolClient},
impl_webpage,
utils::report_error,
Expand Down Expand Up @@ -38,6 +39,9 @@ pub struct Release {
description: Option<String>,
target_name: Option<String>,
rustdoc_status: bool,
build_status: bool,
is_library: bool,
deployment_pending: bool,
pub(crate) build_time: DateTime<Utc>,
stars: i32,
}
Expand All @@ -62,6 +66,7 @@ pub(crate) fn get_releases(
limit: i64,
order: Order,
latest_only: bool,
deployed_only: bool,
) -> Vec<Release> {
let offset = (page - 1) * limit;

Expand All @@ -80,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
Expand All @@ -102,14 +109,18 @@ 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),
description: row.get(2),
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<i32>>(6).unwrap_or(0),
build_status: row.get(7),
is_library: row.get(8),
})
.collect()
}
Expand Down Expand Up @@ -201,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
Expand All @@ -222,9 +235,12 @@ 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),
build_status: row.get("build_status"),
is_library: row.get("is_library"),
},
)
})
Expand Down Expand Up @@ -256,7 +272,14 @@ impl_webpage! {

pub fn home_page(req: &mut Request) -> IronResult<Response> {
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)
}
Expand All @@ -273,7 +296,14 @@ impl_webpage! {

pub fn releases_feed_handler(req: &mut Request) -> IronResult<Response> {
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)
}
Expand Down Expand Up @@ -336,6 +366,7 @@ fn releases_handler(req: &mut Request, release_type: ReleaseType) -> IronResult<
RELEASES_IN_RELEASES,
release_order,
latest_only,
false,
)
};

Expand Down Expand Up @@ -695,7 +726,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};
Expand All @@ -706,6 +738,45 @@ mod tests {
use std::collections::HashSet;
use test_case::test_case;

#[test]
fn get_releases_only_deployed() {
wrapper(|env| {
let db = env.db();

let now = Utc::now();

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<_>>(),
vec!["recent", "old"],
);
assert_eq!(
get_releases(&mut db.conn(), 1, 10, Order::ReleaseTime, true, true)
.iter()
.map(|r| r.name.clone())
.collect::<Vec<_>>(),
vec!["old"],
);
Ok(())
})
}

#[test]
fn get_releases_by_stars() {
wrapper(|env| {
Expand All @@ -724,7 +795,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
Expand All @@ -735,7 +806,6 @@ mod tests {
.map(|release| release.name.as_str())
.collect::<Vec<_>>(),
);

Ok(())
})
}
Expand Down Expand Up @@ -1220,25 +1290,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
Expand All @@ -1254,6 +1343,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",
Expand Down
20 changes: 19 additions & 1 deletion templates/releases/releases.html
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
{{ release.name }}-{{ release.version }}
</div>

<div class="pure-u-1 pure-u-sm-14-24 pure-u-md-16-24 description">
<div class="pure-u-1 pure-u-sm-13-24 pure-u-md-15-24 description">
{{ release.description }}
</div>

Expand All @@ -50,6 +50,24 @@
{{ release.build_time | timeformat(relative=true) }}
</div>
{%- endif %}
{%- if release.rustdoc_status -%}
{%- if release.deployment_pending -%}
<div class="pure-u-1 pure-u-sm-1-24 pure-u-md-1-24 deploymentstatus"
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 }}
</div>
{%- else -%}
<div class="pure-u-1 pure-u-sm-1-24 pure-u-md-1-24 deploymentstatus"
title="deployment finished">
{{ "check" | fas }}
</div>
{%- endif -%}
{%- elif release.is_library and not release.build_status -%}
<div class="pure-u-1 pure-u-sm-1-24 pure-u-md-1-24 deploymentstatus"
title="build error">
{{ "triangle-exclamation" | fas(fw=true) }}
</div>
{%- endif -%}
</div>
</a>
</li>
Expand Down
8 changes: 8 additions & 0 deletions templates/style/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down