Skip to content

Commit dba70e4

Browse files
committed
Update (base update)
[ghstack-poisoned]
1 parent d62e54f commit dba70e4

6 files changed

Lines changed: 303 additions & 201 deletions

File tree

aws/lambda/gha-log-uploader/README.md

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,19 @@ public endpoint. A function URL would not work anyway: those only support the
4545
`RequestResponse` invocation type, so calling one means waiting for
4646
classification to finish.
4747

48-
The payload is just `{"job_id": ..., "repo": "..."}`. `log_classifier` accepts
49-
that shape alongside the API Gateway request its function URL callers send — see
50-
`parse_request` in `../log-classifier/src/main.rs`, whose
51-
`parses_a_direct_invoke_payload` test pins this contract from the other side.
48+
`log_classifier` builds on `lambda_http` with only the `apigw_http` feature, so
49+
`classifier_payload()` reproduces the API Gateway HTTP API v2.0 request it
50+
expects even on a direct invoke. Verified against the deployed function: a
51+
payload with no `job_id` returns its 400 "no job id provided" branch, and a
52+
non-numeric one fails inside its `parse::<usize>()`, which together show both the
53+
envelope and the query string are read.
54+
55+
That envelope is coupling to another lambda's framework, and it would break if
56+
`log_classifier`'s handler changed. Teaching it to accept a plain
57+
`{"job_id", "repo"}` payload is the real fix, and lets its public function URL be
58+
retired once `backfillJobs.mjs`, `keep-going-call-log-classifier` and
59+
`github-status-test` move off it too — worth doing on its own, not as a rider on
60+
this migration.
5261

5362
A failed handoff is logged and reported as `classified: false`, not raised.
5463
Raising would make Lambda retry the whole function, re-downloading a

aws/lambda/gha-log-uploader/lambda_function.py

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,44 @@ def log_object_path(full_name, job_id):
135135
return f"log/{full_name}/{job_id}"
136136

137137

138+
def classifier_payload(full_name, job_id):
139+
"""An API Gateway HTTP API v2.0 request, which is what lambda_http parses.
140+
141+
log_classifier is built on lambda_http with only the `apigw_http` feature, so
142+
it expects this envelope even on a direct invoke. Verified against the
143+
deployed function: a payload with no `job_id` returns its 400 "no job id
144+
provided" branch, and a non-numeric one fails in its `parse::<usize>()`,
145+
which together show both the envelope and the query string are read.
146+
"""
147+
return {
148+
"version": "2.0",
149+
"routeKey": "$default",
150+
"rawPath": "/",
151+
"rawQueryString": f"job_id={job_id}&repo={full_name}",
152+
"headers": {},
153+
"queryStringParameters": {"job_id": str(job_id), "repo": full_name},
154+
"requestContext": {
155+
"accountId": "308535385114",
156+
"apiId": "gha-log-uploader",
157+
"domainName": "lambda-invoke",
158+
"domainPrefix": "lambda-invoke",
159+
"http": {
160+
"method": "GET",
161+
"path": "/",
162+
"protocol": "HTTP/1.1",
163+
"sourceIp": "127.0.0.1",
164+
"userAgent": "gha-log-uploader",
165+
},
166+
"requestId": f"gha-log-uploader-{job_id}",
167+
"routeKey": "$default",
168+
"stage": "$default",
169+
"time": "01/Jan/1970:00:00:00 +0000",
170+
"timeEpoch": 0,
171+
},
172+
"isBase64Encoded": False,
173+
}
174+
175+
138176
def classify_log(full_name, job_id):
139177
"""Kick off classification for a log we just stored. Returns True on handoff.
140178
@@ -149,7 +187,7 @@ def classify_log(full_name, job_id):
149187
lambda_client.invoke(
150188
FunctionName=LOG_CLASSIFIER_FUNCTION,
151189
InvocationType="Event",
152-
Payload=json.dumps({"job_id": job_id, "repo": full_name}).encode(),
190+
Payload=json.dumps(classifier_payload(full_name, job_id)).encode(),
153191
)
154192
return True
155193
except Exception as err:

aws/lambda/gha-log-uploader/test_lambda_function.py

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
import lambda_function
66
from lambda_function import (
7+
classifier_payload,
78
classify_log,
89
download_log,
910
installation_token,
@@ -55,6 +56,23 @@ def test_rejects_a_non_object_payload(self):
5556
parse_event(["pytorch/pytorch", 123])
5657

5758

59+
class TestClassifierPayload(unittest.TestCase):
60+
def test_is_a_v2_request_the_classifier_can_parse(self):
61+
payload = classifier_payload("pytorch/executorch", 999)
62+
# log_classifier builds on lambda_http with only the apigw_http feature,
63+
# so version 2.0 and requestContext.http are what make it deserialize.
64+
self.assertEqual(payload["version"], "2.0")
65+
self.assertIn("http", payload["requestContext"])
66+
self.assertEqual(
67+
payload["queryStringParameters"],
68+
{"job_id": "999", "repo": "pytorch/executorch"},
69+
)
70+
self.assertEqual(payload["rawQueryString"], "job_id=999&repo=pytorch/executorch")
71+
72+
def test_is_json_serializable(self):
73+
json.dumps(classifier_payload("pytorch/pytorch", 1))
74+
75+
5876
class TestClassifyLog(unittest.TestCase):
5977
def test_invokes_the_classifier_asynchronously(self):
6078
with patch.object(lambda_function, "lambda_client") as client:
@@ -65,11 +83,9 @@ def test_invokes_the_classifier_asynchronously(self):
6583
# Event, not RequestResponse: waiting on classification is exactly the
6684
# mistake that gave github-status-test its multi-hundred-second tails.
6785
self.assertEqual(kwargs["InvocationType"], "Event")
68-
# The plain shape log_classifier's parse_request accepts. Its
69-
# parses_a_direct_invoke_payload test pins the other side of this.
7086
self.assertEqual(
71-
json.loads(kwargs["Payload"]),
72-
{"job_id": 123, "repo": "pytorch/pytorch"},
87+
json.loads(kwargs["Payload"])["queryStringParameters"],
88+
{"job_id": "123", "repo": "pytorch/pytorch"},
7389
)
7490

7591
def test_a_failed_invoke_is_reported_not_raised(self):

0 commit comments

Comments
 (0)