From 48fed9568f71c05b36bf16d26caff1bc0110fd4f Mon Sep 17 00:00:00 2001 From: Huy Do Date: Thu, 20 Aug 2026 14:36:33 -0700 Subject: [PATCH] Update [ghstack-poisoned] --- aws/lambda/log-classifier/Cargo.lock | 198 ----------------------- aws/lambda/log-classifier/Cargo.toml | 1 - aws/lambda/log-classifier/src/main.rs | 224 ++++++++++++++++++++++---- 3 files changed, 192 insertions(+), 231 deletions(-) diff --git a/aws/lambda/log-classifier/Cargo.lock b/aws/lambda/log-classifier/Cargo.lock index b0b282ad80..d5ca325f4e 100644 --- a/aws/lambda/log-classifier/Cargo.lock +++ b/aws/lambda/log-classifier/Cargo.lock @@ -23,15 +23,6 @@ version = "0.2.21" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" -[[package]] -name = "android_system_properties" -version = "0.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "819e7219dbd41043ac279b19830f2efc897156490d7fd6ea916720117ee66311" -dependencies = [ - "libc", -] - [[package]] name = "ansi_term" version = "0.12.1" @@ -583,36 +574,12 @@ dependencies = [ "tracing", ] -[[package]] -name = "aws_lambda_events" -version = "0.6.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "55d7e5deac5e49330042b4e174dafe84ebf71685bfcd94f285bac7aa31e0aeb1" -dependencies = [ - "base64 0.13.1", - "bytes", - "chrono", - "http 0.2.12", - "http-body 0.4.6", - "http-serde", - "query_map", - "serde", - "serde_derive", - "serde_json", -] - [[package]] name = "base16ct" version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "349a06037c7bf932dd7e7d1f653678b2038b9ad46a74102f1fc7bd7872678cce" -[[package]] -name = "base64" -version = "0.13.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9e1b586273c5702936fe7b7d6896644d8be71e6314cfe09d3167c95f712589e8" - [[package]] name = "base64" version = "0.21.7" @@ -667,9 +634,6 @@ name = "bytes" version = "1.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1e748733b7cbc798e1434b6ac524f0c1ff2ab456fe201501e6497c8417a4fc33" -dependencies = [ - "serde", -] [[package]] name = "bytes-utils" @@ -699,20 +663,6 @@ version = "1.0.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9330f8b2ff13f34540b44e946ef35111825727b38d33286ef986142615121801" -[[package]] -name = "chrono" -version = "0.4.44" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c673075a2e0e5f4a1dde27ce9dee1ea4558c7ffe648f576438a20ca1d2acc4b0" -dependencies = [ - "iana-time-zone", - "js-sys", - "num-traits", - "serde", - "wasm-bindgen", - "windows-link", -] - [[package]] name = "cmake" version = "0.1.57" @@ -949,15 +899,6 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "34aa73646ffb006b8f5147f3dc182bd4bcb190227ce861fc4a4844bf8e3cb2c0" -[[package]] -name = "encoding_rs" -version = "0.8.35" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "75030f3c4f45dafd7586dd6780965a8c7e8e285a5ecb86713e63a79c5b2766f3" -dependencies = [ - "cfg-if", -] - [[package]] name = "equivalent" version = "1.0.2" @@ -1333,16 +1274,6 @@ dependencies = [ "pin-project-lite", ] -[[package]] -name = "http-serde" -version = "1.1.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6f560b665ad9f1572cfcaf034f7fb84338a7ce945216d64a90fd81f046a3caee" -dependencies = [ - "http 0.2.12", - "serde", -] - [[package]] name = "httparse" version = "1.10.1" @@ -1456,30 +1387,6 @@ dependencies = [ "tracing", ] -[[package]] -name = "iana-time-zone" -version = "0.1.65" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e31bc9ad994ba00e440a8aa5c9ef0ec67d5cb5e5cb0cc7f8b744a35b389cc470" -dependencies = [ - "android_system_properties", - "core-foundation-sys", - "iana-time-zone-haiku", - "js-sys", - "log", - "wasm-bindgen", - "windows-core", -] - -[[package]] -name = "iana-time-zone-haiku" -version = "0.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f31827a206f56af32e590ba56d5d2d085f558508192593743f16b2306495269f" -dependencies = [ - "cc", -] - [[package]] name = "icu_collections" version = "2.1.1" @@ -1647,28 +1554,6 @@ dependencies = [ "wasm-bindgen", ] -[[package]] -name = "lambda_http" -version = "0.6.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9b1881c01539f63a1d20ef0a1915931547fdbbbf3b5e57a3b731ddbe9611b043" -dependencies = [ - "aws_lambda_events", - "base64 0.13.1", - "bytes", - "encoding_rs", - "http 0.2.12", - "http-body 0.4.6", - "hyper 0.14.32", - "lambda_runtime", - "mime", - "query_map", - "serde", - "serde_json", - "serde_urlencoded", - "url", -] - [[package]] name = "lambda_runtime" version = "0.6.1" @@ -1762,7 +1647,6 @@ dependencies = [ "flate2", "http 0.2.12", "insta", - "lambda_http", "lambda_runtime", "native-tls", "once_cell", @@ -1804,12 +1688,6 @@ version = "2.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" -[[package]] -name = "mime" -version = "0.3.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6877bb514081ee2a7ff5ef9de3281f14a4dd4bceac4c09388074a6b5df8a139a" - [[package]] name = "miniz_oxide" version = "0.8.9" @@ -2103,17 +1981,6 @@ dependencies = [ "unicode-ident", ] -[[package]] -name = "query_map" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fe3212d819cbdcce67f786cdaf3fe0c2e9d09a6dcd9c9367a1bd344135b8c809" -dependencies = [ - "form_urlencoded", - "serde", - "serde_derive", -] - [[package]] name = "quote" version = "1.0.45" @@ -2459,18 +2326,6 @@ dependencies = [ "zmij", ] -[[package]] -name = "serde_urlencoded" -version = "0.7.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d3491c14715ca2294c4d6a88f15e84739788c1d030eed8c110436aafdaa2f3fd" -dependencies = [ - "form_urlencoded", - "itoa", - "ryu", - "serde", -] - [[package]] name = "sha1" version = "0.10.6" @@ -3067,65 +2922,12 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" -[[package]] -name = "windows-core" -version = "0.62.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b8e83a14d34d0623b51dce9581199302a221863196a1dde71a7663a4c2be9deb" -dependencies = [ - "windows-implement", - "windows-interface", - "windows-link", - "windows-result", - "windows-strings", -] - -[[package]] -name = "windows-implement" -version = "0.60.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "053e2e040ab57b9dc951b72c264860db7eb3b0200ba345b4e4c3b14f67855ddf" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] - -[[package]] -name = "windows-interface" -version = "0.59.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3f316c4a2570ba26bbec722032c4099d8c8bc095efccdc15688708623367e358" -dependencies = [ - "proc-macro2", - "quote", - "syn", -] - [[package]] name = "windows-link" version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f0805222e57f7521d6a62e36fa9163bc891acd422f971defe97d64e70d0a4fe5" -[[package]] -name = "windows-result" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7781fa89eaf60850ac3d2da7af8e5242a5ea78d1a11c49bf2910bb5a73853eb5" -dependencies = [ - "windows-link", -] - -[[package]] -name = "windows-strings" -version = "0.5.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7837d08f69c77cf6b07689544538e017c1bfcf57e34b4c0ff58e6c2cd3b37091" -dependencies = [ - "windows-link", -] - [[package]] name = "windows-sys" version = "0.52.0" diff --git a/aws/lambda/log-classifier/Cargo.toml b/aws/lambda/log-classifier/Cargo.toml index 6ed2feca7d..ba0298a861 100644 --- a/aws/lambda/log-classifier/Cargo.toml +++ b/aws/lambda/log-classifier/Cargo.toml @@ -4,7 +4,6 @@ version = "0.1.0" edition = "2021" [dependencies] -lambda_http = { version = "0.6.0", default-features = false, features = ["apigw_http"] } lambda_runtime = "0.6.0" tracing = { version = "0.1", features = ["log"] } tracing-subscriber = { version = "0.3", default-features = false, features = ["fmt"] } diff --git a/aws/lambda/log-classifier/src/main.rs b/aws/lambda/log-classifier/src/main.rs index 5aa7cbbb37..ac2c07fe63 100644 --- a/aws/lambda/log-classifier/src/main.rs +++ b/aws/lambda/log-classifier/src/main.rs @@ -1,4 +1,5 @@ -use lambda_http::{run, service_fn, Body, Error, IntoResponse, Request, RequestExt, Response}; +use lambda_runtime::{run, service_fn, Error, LambdaEvent}; +use serde_json::{json, Value}; use anyhow::Result; use std::time::Instant; @@ -78,41 +79,97 @@ async fn handle( } } -async fn function_handler(event: Request) -> Result, Error> { - // Extract some useful information from the request - let query_string_parameters = event.query_string_parameters(); - Ok(match query_string_parameters.first("job_id") { - Some(job_id) => { - let job_id = job_id.parse::()?; - let repo = query_string_parameters - .first("repo") - .unwrap_or_else(|| "pytorch/pytorch"); - let context_depth = query_string_parameters - .first("context_depth") - .unwrap_or_else(|| CONTEXT_DEPTH) - .parse::()?; - let is_temp_log = query_string_parameters - .first("temp_log") - .map_or(false, |v| v == "true"); - handle( - job_id, - repo, - ShouldWriteDynamo(true), - context_depth, - is_temp_log, - ) - .await? - .into_response() - .await - } +/// What the handler needs, however the caller chose to say it. +#[derive(Debug, PartialEq)] +struct ClassifyRequest { + job_id: usize, + repo: String, + context_depth: usize, + is_temp_log: bool, +} + +/// Pull a single parameter out of either payload shape. +/// +/// Two callers exist. Function URL callers (backfillJobs.mjs, +/// keep-going-call-log-classifier, github-status-test) send an API Gateway HTTP +/// API v2.0 request, where the values live under `queryStringParameters` and are +/// always strings. Direct `lambda:InvokeFunction` callers (gha-log-uploader) send +/// a plain `{"job_id": 123, "repo": "..."}` object, where `job_id` is a real +/// number. Accepting both is what lets an async invoke skip the public function +/// URL without every caller having to synthesise an HTTP request. +fn param(event: &Value, name: &str) -> Option { + let from_query = event + .get("queryStringParameters") + .and_then(|q| q.get(name)) + .and_then(|v| v.as_str()) + .map(str::to_string); + if from_query.is_some() { + return from_query; + } - _ => Response::builder() - .status(400) - .body("no job id provided".into()) - .expect("failed to render response"), + // A v2.0 request with no `queryStringParameters` still carries the raw + // string, so fall back to it rather than 400-ing a well-formed request. + let from_raw = event + .get("rawQueryString") + .and_then(|v| v.as_str()) + .and_then(|raw| { + raw.split('&') + .filter_map(|pair| pair.split_once('=')) + .find(|(k, _)| *k == name) + .map(|(_, v)| v.to_string()) + }); + if from_raw.is_some() { + return from_raw; + } + + match event.get(name) { + Some(Value::String(s)) => Some(s.clone()), + Some(Value::Number(n)) => Some(n.to_string()), + Some(Value::Bool(b)) => Some(b.to_string()), + _ => None, + } +} + +fn parse_request(event: &Value) -> Option { + let job_id = param(event, "job_id")?.parse::().ok()?; + Some(ClassifyRequest { + job_id, + repo: param(event, "repo").unwrap_or_else(|| "pytorch/pytorch".to_string()), + context_depth: param(event, "context_depth") + .and_then(|v| v.parse::().ok()) + .unwrap_or_else(|| CONTEXT_DEPTH.parse::().expect("valid default")), + is_temp_log: param(event, "temp_log").map_or(false, |v| v == "true"), }) } +/// The API Gateway response shape, kept so function URL callers see exactly what +/// they saw when this was a lambda_http handler. +fn response(status: u16, body: impl Into) -> Value { + json!({ + "statusCode": status, + "headers": {}, + "body": body.into(), + "isBase64Encoded": false, + }) +} + +async fn function_handler(event: LambdaEvent) -> Result { + let Some(request) = parse_request(&event.payload) else { + return Ok(response(400, "no job id provided")); + }; + + let body = handle( + request.job_id, + &request.repo, + ShouldWriteDynamo(true), + request.context_depth, + request.is_temp_log, + ) + .await?; + + Ok(response(200, body)) +} + #[tokio::main] async fn main() -> Result<(), Error> { tracing_subscriber::fmt() @@ -131,6 +188,109 @@ mod test { use log_classifier::rule::Rule; use regex::Regex; + fn v2_request(query: Value) -> Value { + json!({ + "version": "2.0", + "routeKey": "$default", + "rawPath": "/", + "headers": {}, + "queryStringParameters": query, + "isBase64Encoded": false, + }) + } + + #[test] + fn parses_a_direct_invoke_payload() { + // gha-log-uploader sends this: job_id is a real number, not a string. + assert_eq!( + parse_request(&json!({"job_id": 123, "repo": "pytorch/executorch"})), + Some(ClassifyRequest { + job_id: 123, + repo: "pytorch/executorch".to_string(), + context_depth: 12, + is_temp_log: false, + }) + ); + } + + #[test] + fn parses_a_function_url_request() { + // What backfillJobs.mjs and keep-going-call-log-classifier send. + assert_eq!( + parse_request(&v2_request( + json!({"job_id": "123", "repo": "pytorch/pytorch", "temp_log": "true"}) + )), + Some(ClassifyRequest { + job_id: 123, + repo: "pytorch/pytorch".to_string(), + context_depth: 12, + is_temp_log: true, + }) + ); + } + + #[test] + fn falls_back_to_the_raw_query_string() { + let event = json!({ + "version": "2.0", + "rawQueryString": "job_id=99&repo=pytorch/rl&context_depth=3", + }); + let parsed = parse_request(&event).expect("should parse"); + assert_eq!(parsed.job_id, 99); + assert_eq!(parsed.repo, "pytorch/rl"); + assert_eq!(parsed.context_depth, 3); + } + + #[test] + fn query_parameters_win_over_a_top_level_key() { + // A v2.0 envelope has no top-level job_id, but if one ever appears the + // request the caller actually made is the one to honour. + let mut event = v2_request(json!({"job_id": "1"})); + event["job_id"] = json!(2); + assert_eq!(parse_request(&event).expect("should parse").job_id, 1); + } + + #[test] + fn defaults_repo_and_context_depth() { + let parsed = parse_request(&json!({"job_id": 5})).expect("should parse"); + assert_eq!(parsed.repo, "pytorch/pytorch"); + assert_eq!(parsed.context_depth, 12); + assert!(!parsed.is_temp_log); + } + + #[test] + fn rejects_a_payload_with_no_job_id() { + assert_eq!(parse_request(&json!({"repo": "pytorch/pytorch"})), None); + assert_eq!(parse_request(&v2_request(json!({}))), None); + } + + #[test] + fn rejects_a_non_numeric_job_id() { + assert_eq!(parse_request(&json!({"job_id": "not a number"})), None); + } + + #[test] + fn temp_log_is_only_true_for_the_exact_string() { + // It arrives as a string over the function URL and could arrive as a + // bool on a direct invoke; both normalise through param(). + assert!(parse_request(&json!({"job_id": 1, "temp_log": true})) + .expect("should parse") + .is_temp_log); + assert!(!parse_request(&json!({"job_id": 1, "temp_log": "false"})) + .expect("should parse") + .is_temp_log); + } + + #[test] + fn response_keeps_the_api_gateway_shape() { + // Function URL callers still get a structured response, unchanged from + // when this was a lambda_http handler. + let r = response(400, "no job id provided"); + assert_eq!(r["statusCode"], 400); + assert_eq!(r["body"], "no job id provided"); + assert_eq!(r["isBase64Encoded"], false); + } + #[test] fn basic_evaluate_rule() { let rule = Rule {