Skip to content

Commit 8dfb8bf

Browse files
Merge pull request #1494 from rust-lang/gh-comment-auth-fix
Split GraphQL client from the REST API GitHub client and use proper authentication for it
2 parents c79fd0e + 1542b73 commit 8dfb8bf

File tree

3 files changed

+52
-19
lines changed

3 files changed

+52
-19
lines changed

site/src/github.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ use serde::Deserialize;
99
type BoxedError = Box<dyn std::error::Error + Send + Sync>;
1010

1111
pub const RUST_REPO_GITHUB_API_URL: &str = "https://api.github.com/repos/rust-lang/rust";
12-
pub const RUST_REPO_GITHUB_GRAPH_URL: &str = "https://api.github.com/graphql";
1312

1413
/// Comments that are temporary and do not add any value once there has been a new development
1514
/// (a rustc build or a perf. run was finished) are marked with this comment.

site/src/github/client.rs

Lines changed: 50 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,13 @@
11
use anyhow::Context;
22
use chrono::{DateTime, Utc};
3+
use http::header;
34
use http::header::USER_AGENT;
45
use serde::de::DeserializeOwned;
56

67
use crate::{api::github::Issue, load::SiteCtxt};
78

9+
const BOT_USER_AGENT: &str = "perf-rust-lang-org-server";
10+
811
/// A client for interacting with the GitHub API
912
pub struct Client {
1013
repository_url: String,
@@ -227,6 +230,49 @@ impl Client {
227230
}
228231
}
229232

233+
async fn send(
234+
&self,
235+
request: reqwest::RequestBuilder,
236+
) -> Result<reqwest::Response, reqwest::Error> {
237+
request
238+
.header(USER_AGENT, BOT_USER_AGENT)
239+
.basic_auth("rust-timer", Some(&self.token))
240+
.send()
241+
.await
242+
}
243+
}
244+
245+
const GRAPHQL_API_BASE: &str = "https://api.github.com/graphql";
246+
247+
/// A client for interacting with the GraphQL GitHub API.
248+
pub struct GraphQLClient {
249+
inner: reqwest::Client,
250+
}
251+
252+
impl GraphQLClient {
253+
/// Create a GraphQL client from a `SiteCtxt`.
254+
pub fn from_ctxt(ctxt: &SiteCtxt) -> Self {
255+
let token = ctxt
256+
.config
257+
.keys
258+
.github_api_token
259+
.clone()
260+
.expect("needs github API token");
261+
262+
let mut headers = header::HeaderMap::new();
263+
headers.insert(USER_AGENT, header::HeaderValue::from_static(BOT_USER_AGENT));
264+
headers.insert(
265+
header::AUTHORIZATION,
266+
header::HeaderValue::from_str(&format!("token {}", token)).unwrap(),
267+
);
268+
269+
let client = reqwest::ClientBuilder::new()
270+
.default_headers(headers)
271+
.build()
272+
.unwrap();
273+
Self { inner: client }
274+
}
275+
230276
pub async fn get_comments(&self, pull_request: u32) -> anyhow::Result<Vec<ResponseComment>> {
231277
const QUERY: &str = "query($owner: String!, $repo: String!, $pr: Int!, $cursor: String) {
232278
repository(owner: $owner, name: $repo) {
@@ -273,7 +319,7 @@ impl Client {
273319
let mut cursor = None;
274320
loop {
275321
let mut resp: Response = self
276-
.graphql(
322+
.send(
277323
QUERY,
278324
serde_json::json!({
279325
"owner": owner,
@@ -303,7 +349,7 @@ impl Client {
303349
}
304350
}";
305351

306-
self.graphql::<Option<MinimizeData>, _>(
352+
self.send::<Option<MinimizeData>, _>(
307353
MINIMIZE,
308354
serde_json::json!({
309355
"node_id": comment_id,
@@ -314,7 +360,7 @@ impl Client {
314360
Ok(())
315361
}
316362

317-
async fn graphql<T: DeserializeOwned, V: serde::Serialize>(
363+
async fn send<T: DeserializeOwned, V: serde::Serialize>(
318364
&self,
319365
query: &str,
320366
variables: V,
@@ -327,7 +373,7 @@ impl Client {
327373

328374
let response: GraphResponse<T> = self
329375
.inner
330-
.post(&self.repository_url)
376+
.post(GRAPHQL_API_BASE)
331377
.json(&GraphPayload { query, variables })
332378
.send()
333379
.await?
@@ -344,17 +390,6 @@ impl Client {
344390
))
345391
}
346392
}
347-
348-
async fn send(
349-
&self,
350-
request: reqwest::RequestBuilder,
351-
) -> Result<reqwest::Response, reqwest::Error> {
352-
request
353-
.header(USER_AGENT, "perf-rust-lang-org-server")
354-
.basic_auth("rust-timer", Some(&self.token))
355-
.send()
356-
.await
357-
}
358393
}
359394

360395
#[derive(serde::Deserialize)]

site/src/github/comparison_summary.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::load::SiteCtxt;
66

77
use database::{ArtifactId, QueuedCommit};
88

9-
use crate::github::{COMMENT_MARK_TEMPORARY, RUST_REPO_GITHUB_API_URL, RUST_REPO_GITHUB_GRAPH_URL};
9+
use crate::github::{COMMENT_MARK_TEMPORARY, RUST_REPO_GITHUB_API_URL};
1010
use std::collections::HashSet;
1111
use std::fmt::Write;
1212

@@ -83,8 +83,7 @@ async fn post_comparison_comment(
8383

8484
client.post_comment(pr, body).await;
8585

86-
let graph_client =
87-
super::client::Client::from_ctxt(ctxt, RUST_REPO_GITHUB_GRAPH_URL.to_owned());
86+
let graph_client = super::client::GraphQLClient::from_ctxt(ctxt);
8887
for comment in graph_client.get_comments(pr).await? {
8988
// If this bot is the author of the comment, the comment is not yet minimized and it is
9089
// a temporary comment, minimize it.

0 commit comments

Comments
 (0)