Skip to content

Hide old rust-timer comments when a summary is posted #1490

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

Merged
merged 2 commits into from
Nov 4, 2022
Merged
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
10 changes: 10 additions & 0 deletions site/src/github.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ use serde::Deserialize;

type BoxedError = Box<dyn std::error::Error + Send + Sync>;

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 = "<!-- rust-timer: temporary -->";

pub use comparison_summary::post_finished;

/// Enqueues try build artifacts and posts a message about them on the original rollup PR
Expand Down Expand Up @@ -234,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(())
}
Expand Down
147 changes: 147 additions & 0 deletions site/src/github/client.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -226,6 +227,124 @@ impl Client {
}
}

pub async fn get_comments(&self, pull_request: u32) -> anyhow::Result<Vec<ResponseComment>> {
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<ResponseComment>,
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::<Option<MinimizeData>, _>(
MINIMIZE,
serde_json::json!({
"node_id": comment_id,
"reason": reason,
}),
)
.await?;
Ok(())
}

async fn graphql<T: DeserializeOwned, V: serde::Serialize>(
&self,
query: &str,
variables: V,
) -> anyhow::Result<T> {
#[derive(serde::Serialize)]
struct GraphPayload<'a, V> {
query: &'a str,
variables: V,
}

let response: GraphResponse<T> = 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,
Expand All @@ -238,6 +357,34 @@ impl Client {
}
}

#[derive(serde::Deserialize)]
struct GraphResponse<T> {
data: T,
#[serde(default)]
errors: Vec<GraphError>,
}

#[derive(Debug, serde::Deserialize)]
struct GraphError {
message: String,
}

#[derive(Debug, serde::Deserialize)]
#[serde(rename_all = "camelCase")]
struct GraphPageInfo {
end_cursor: Option<String>,
}

#[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,
Expand Down
32 changes: 26 additions & 6 deletions site/src/github/comparison_summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use crate::load::SiteCtxt;

use database::{ArtifactId, QueuedCommit};

use crate::github::{COMMENT_MARK_TEMPORARY, RUST_REPO_GITHUB_API_URL, RUST_REPO_GITHUB_GRAPH_URL};
use std::collections::HashSet;
use std::fmt::Write;

Expand Down Expand Up @@ -57,26 +58,45 @@ 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:?}");
}
}
}
}

/// 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) {
let client = super::client::Client::from_ctxt(
ctxt,
"https://api.github.com/repos/rust-lang/rust".to_owned(),
);
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 {
Ok(message) => message,
Err(error) => error,
};

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 {
Expand Down
28 changes: 16 additions & 12 deletions site/src/request_handlers/github.rs
Original file line number Diff line number Diff line change
@@ -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,
COMMENT_MARK_TEMPORARY, RUST_REPO_GITHUB_API_URL,
};
use crate::load::SiteCtxt;

use std::sync::Arc;
Expand Down Expand Up @@ -29,10 +32,7 @@ async fn handle_push(ctxt: Arc<SiteCtxt>, push: github::Push) -> ServerResult<gi
&ctxt,
"https://api.github.com/repos/rust-lang-ci/rust".to_owned(),
);
let main_repo_client = client::Client::from_ctxt(
&ctxt,
"https://api.github.com/repos/rust-lang/rust".to_owned(),
);
let main_repo_client = client::Client::from_ctxt(&ctxt, RUST_REPO_GITHUB_API_URL.to_owned());
if push.r#ref != "refs/heads/master" || push.sender.login != "bors" {
return Ok(github::Response);
}
Expand Down Expand Up @@ -70,10 +70,7 @@ async fn handle_issue(
issue: github::Issue,
comment: github::Comment,
) -> ServerResult<github::Response> {
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(),
Expand Down Expand Up @@ -112,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);
Expand All @@ -129,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);
Expand Down