From 859151569f73e52ec6e152bb788e215ea4e28c1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 3 Nov 2022 18:21:55 +0100 Subject: [PATCH 1/2] Introduce a constant for GitHub API URL --- site/src/github.rs | 2 ++ site/src/github/comparison_summary.rs | 6 ++---- site/src/request_handlers/github.rs | 15 ++++++--------- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/site/src/github.rs b/site/src/github.rs index b5f9dc4ce..553360196 100644 --- a/site/src/github.rs +++ b/site/src/github.rs @@ -8,6 +8,8 @@ use serde::Deserialize; type BoxedError = Box; +pub const RUST_REPO_GITHUB_API_URL: &str = "https://api.github.com/repos/rust-lang/rust"; + pub use comparison_summary::post_finished; /// Enqueues try build artifacts and posts a message about them on the original rollup PR diff --git a/site/src/github/comparison_summary.rs b/site/src/github/comparison_summary.rs index c7925387d..935c9933a 100644 --- a/site/src/github/comparison_summary.rs +++ b/site/src/github/comparison_summary.rs @@ -6,6 +6,7 @@ use crate::load::SiteCtxt; use database::{ArtifactId, QueuedCommit}; +use crate::github::RUST_REPO_GITHUB_API_URL; use std::collections::HashSet; use std::fmt::Write; @@ -66,10 +67,7 @@ pub async fn post_finished(ctxt: &SiteCtxt) { /// /// `is_master_commit` is used to differentiate messages for try runs and post-merge runs. async fn post_comparison_comment(ctxt: &SiteCtxt, commit: QueuedCommit, is_master_commit: bool) { - let client = super::client::Client::from_ctxt( - ctxt, - "https://api.github.com/repos/rust-lang/rust".to_owned(), - ); + let client = super::client::Client::from_ctxt(ctxt, RUST_REPO_GITHUB_API_URL.to_owned()); let pr = commit.pr; let body = match summarize_run(ctxt, commit, is_master_commit).await { Ok(message) => message, diff --git a/site/src/request_handlers/github.rs b/site/src/request_handlers/github.rs index b6ed18ac0..644b18726 100644 --- a/site/src/request_handlers/github.rs +++ b/site/src/request_handlers/github.rs @@ -1,5 +1,8 @@ use crate::api::{github, ServerResult}; -use crate::github::{client, enqueue_shas, parse_homu_comment, rollup_pr_number, unroll_rollup}; +use crate::github::{ + client, enqueue_shas, parse_homu_comment, rollup_pr_number, unroll_rollup, + RUST_REPO_GITHUB_API_URL, +}; use crate::load::SiteCtxt; use std::sync::Arc; @@ -29,10 +32,7 @@ async fn handle_push(ctxt: Arc, push: github::Push) -> ServerResult ServerResult { - let main_client = client::Client::from_ctxt( - &ctxt, - "https://api.github.com/repos/rust-lang/rust".to_owned(), - ); + let main_client = client::Client::from_ctxt(&ctxt, RUST_REPO_GITHUB_API_URL.to_owned()); let ci_client = client::Client::from_ctxt( &ctxt, "https://api.github.com/repos/rust-lang-ci/rust".to_owned(), From 89663360224b7367ad7a9601c9e4d72f19266053 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Ber=C3=A1nek?= Date: Thu, 3 Nov 2022 21:17:33 +0100 Subject: [PATCH 2/2] Minimize all `rustc-timer` non-summary comments after a summary is posted --- site/src/github.rs | 8 ++ site/src/github/client.rs | 147 ++++++++++++++++++++++++++ site/src/github/comparison_summary.rs | 28 ++++- site/src/request_handlers/github.rs | 15 ++- 4 files changed, 191 insertions(+), 7 deletions(-) diff --git a/site/src/github.rs b/site/src/github.rs index 553360196..d5a6873cf 100644 --- a/site/src/github.rs +++ b/site/src/github.rs @@ -9,6 +9,13 @@ use serde::Deserialize; type BoxedError = Box; pub const RUST_REPO_GITHUB_API_URL: &str = "https://api.github.com/repos/rust-lang/rust"; +pub const RUST_REPO_GITHUB_GRAPH_URL: &str = "https://api.github.com/graphql"; + +/// Comments that are temporary and do not add any value once there has been a new development +/// (a rustc build or a perf. run was finished) are marked with this comment. +/// +/// They are removed once a perf. run comparison summary is posted on a PR. +pub const COMMENT_MARK_TEMPORARY: &str = ""; pub use comparison_summary::post_finished; @@ -236,6 +243,7 @@ pub async fn enqueue_shas( )); } } + msg.push_str(&format!("\n{COMMENT_MARK_TEMPORARY}")); main_client.post_comment(pr_number, msg).await; Ok(()) } diff --git a/site/src/github/client.rs b/site/src/github/client.rs index f0868fb58..4dbfa2143 100644 --- a/site/src/github/client.rs +++ b/site/src/github/client.rs @@ -1,6 +1,7 @@ use anyhow::Context; use chrono::{DateTime, Utc}; use http::header::USER_AGENT; +use serde::de::DeserializeOwned; use crate::{api::github::Issue, load::SiteCtxt}; @@ -226,6 +227,124 @@ impl Client { } } + pub async fn get_comments(&self, pull_request: u32) -> anyhow::Result> { + const QUERY: &str = "query($owner: String!, $repo: String!, $pr: Int!, $cursor: String) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $pr) { + comments(first: 100, after: $cursor) { + nodes { + id + body + isMinimized + viewerDidAuthor + } + pageInfo { + endCursor + } + } + } + } + }"; + + #[derive(Debug, serde::Deserialize)] + struct Response { + repository: ResponseRepo, + } + #[derive(Debug, serde::Deserialize)] + #[serde(rename_all = "camelCase")] + struct ResponseRepo { + pull_request: ResponsePR, + } + #[derive(Debug, serde::Deserialize)] + struct ResponsePR { + comments: ResponseComments, + } + #[derive(Debug, serde::Deserialize)] + #[serde(rename_all = "camelCase")] + struct ResponseComments { + nodes: Vec, + page_info: GraphPageInfo, + } + + let owner = "rust-lang"; + let repo = "rust"; + + let mut comments = Vec::new(); + let mut cursor = None; + loop { + let mut resp: Response = self + .graphql( + QUERY, + serde_json::json!({ + "owner": owner, + "repo": repo, + "pr": pull_request, + "cursor": cursor, + }), + ) + .await?; + cursor = resp.repository.pull_request.comments.page_info.end_cursor; + comments.append(&mut resp.repository.pull_request.comments.nodes); + + if cursor.is_none() { + break; + } + } + Ok(comments) + } + + pub async fn hide_comment(&self, comment_id: &str, reason: &str) -> anyhow::Result<()> { + #[derive(serde::Deserialize)] + struct MinimizeData {} + + const MINIMIZE: &str = "mutation($node_id: ID!, $reason: ReportedContentClassifiers!) { + minimizeComment(input: {subjectId: $node_id, classifier: $reason}) { + __typename + } + }"; + + self.graphql::, _>( + MINIMIZE, + serde_json::json!({ + "node_id": comment_id, + "reason": reason, + }), + ) + .await?; + Ok(()) + } + + async fn graphql( + &self, + query: &str, + variables: V, + ) -> anyhow::Result { + #[derive(serde::Serialize)] + struct GraphPayload<'a, V> { + query: &'a str, + variables: V, + } + + let response: GraphResponse = self + .inner + .post(&self.repository_url) + .json(&GraphPayload { query, variables }) + .send() + .await? + .error_for_status()? + .json() + .await?; + + if response.errors.is_empty() { + Ok(response.data) + } else { + Err(anyhow::anyhow!( + "GraphQL query failed: {}", + response.errors[0].message + )) + } + } + async fn send( &self, request: reqwest::RequestBuilder, @@ -238,6 +357,34 @@ impl Client { } } +#[derive(serde::Deserialize)] +struct GraphResponse { + data: T, + #[serde(default)] + errors: Vec, +} + +#[derive(Debug, serde::Deserialize)] +struct GraphError { + message: String, +} + +#[derive(Debug, serde::Deserialize)] +#[serde(rename_all = "camelCase")] +struct GraphPageInfo { + end_cursor: Option, +} + +#[derive(Debug, serde::Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ResponseComment { + pub id: String, + pub body: String, + pub is_minimized: bool, + // Is the account that fetches this comment also the original author of the comment? + pub viewer_did_author: bool, +} + #[derive(Debug, serde::Deserialize)] pub struct CreatePrResponse { pub number: u32, diff --git a/site/src/github/comparison_summary.rs b/site/src/github/comparison_summary.rs index 935c9933a..6d76842ee 100644 --- a/site/src/github/comparison_summary.rs +++ b/site/src/github/comparison_summary.rs @@ -6,7 +6,7 @@ use crate::load::SiteCtxt; use database::{ArtifactId, QueuedCommit}; -use crate::github::RUST_REPO_GITHUB_API_URL; +use crate::github::{COMMENT_MARK_TEMPORARY, RUST_REPO_GITHUB_API_URL, RUST_REPO_GITHUB_GRAPH_URL}; use std::collections::HashSet; use std::fmt::Write; @@ -58,7 +58,10 @@ pub async fn post_finished(ctxt: &SiteCtxt) { assert_eq!(completed, queued_commit); let is_master_commit = master_commits.contains(&queued_commit.sha); - post_comparison_comment(ctxt, queued_commit, is_master_commit).await; + if let Err(error) = post_comparison_comment(ctxt, queued_commit, is_master_commit).await + { + log::error!("Cannot post comparison comment: {error:?}"); + } } } } @@ -66,7 +69,11 @@ pub async fn post_finished(ctxt: &SiteCtxt) { /// Posts a comment to GitHub summarizing the comparison of the queued commit with its parent /// /// `is_master_commit` is used to differentiate messages for try runs and post-merge runs. -async fn post_comparison_comment(ctxt: &SiteCtxt, commit: QueuedCommit, is_master_commit: bool) { +async fn post_comparison_comment( + ctxt: &SiteCtxt, + commit: QueuedCommit, + is_master_commit: bool, +) -> anyhow::Result<()> { let client = super::client::Client::from_ctxt(ctxt, RUST_REPO_GITHUB_API_URL.to_owned()); let pr = commit.pr; let body = match summarize_run(ctxt, commit, is_master_commit).await { @@ -75,6 +82,21 @@ async fn post_comparison_comment(ctxt: &SiteCtxt, commit: QueuedCommit, is_maste }; client.post_comment(pr, body).await; + + let graph_client = + super::client::Client::from_ctxt(ctxt, RUST_REPO_GITHUB_GRAPH_URL.to_owned()); + for comment in graph_client.get_comments(pr).await? { + // If this bot is the author of the comment, the comment is not yet minimized and it is + // a temporary comment, minimize it. + if comment.viewer_did_author + && !comment.is_minimized + && comment.body.contains(COMMENT_MARK_TEMPORARY) + { + log::debug!("Hiding comment {}", comment.id); + graph_client.hide_comment(&comment.id, "OUTDATED").await?; + } + } + Ok(()) } fn make_comparison_url(commit: &QueuedCommit, stat: Metric) -> String { diff --git a/site/src/request_handlers/github.rs b/site/src/request_handlers/github.rs index 644b18726..f0ca9e83a 100644 --- a/site/src/request_handlers/github.rs +++ b/site/src/request_handlers/github.rs @@ -1,7 +1,7 @@ use crate::api::{github, ServerResult}; use crate::github::{ client, enqueue_shas, parse_homu_comment, rollup_pr_number, unroll_rollup, - RUST_REPO_GITHUB_API_URL, + COMMENT_MARK_TEMPORARY, RUST_REPO_GITHUB_API_URL, }; use crate::load::SiteCtxt; @@ -109,7 +109,10 @@ async fn handle_rust_timer( main_client .post_comment( issue.number, - "Insufficient permissions to issue commands to rust-timer.", + format!( + "Insufficient permissions to issue commands to rust-timer. +{COMMENT_MARK_TEMPORARY}" + ), ) .await; return Ok(github::Response); @@ -126,9 +129,13 @@ async fn handle_rust_timer( main_client .post_comment( issue.number, - "Awaiting bors try build completion. + format!( + "Awaiting bors try build completion. -@rustbot label: +S-waiting-on-perf", +@rustbot label: +S-waiting-on-perf + +{COMMENT_MARK_TEMPORARY}" + ), ) .await; return Ok(github::Response);