-
Notifications
You must be signed in to change notification settings - Fork 142
Stop re-implementing the log upload in backfill_events.py #8596
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
base: gh/huydhn/7/base
Are you sure you want to change the base?
Changes from all commits
095f94d
07ba0c3
ab23f3f
15902f6
8cb058f
fd5d498
eac148a
560b5e8
c8b7384
5a2abf5
a8b8432
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,77 +1,70 @@ | ||
| #!/usr/bin/env python3 | ||
|
|
||
| import gzip | ||
| import json | ||
| import os | ||
| from typing import Any | ||
| from urllib.request import urlopen | ||
| from urllib.error import HTTPError | ||
| from urllib.request import Request, urlopen | ||
| from warnings import warn | ||
|
|
||
| import boto3 | ||
| from octokit import Octokit | ||
|
|
||
|
|
||
| S3 = boto3.resource("s3") | ||
| BUCKET_NAME = "ossci-raw-job-status" | ||
| BUCKET = S3.Bucket(BUCKET_NAME) | ||
|
|
||
| DYNAMO = boto3.resource("dynamodb") | ||
|
|
||
|
|
||
| def json_dumps(body: Any) -> str: | ||
| # This logic is copied from github-status-test lambda function | ||
| return json.dumps(body, sort_keys=True, indent=4, separators=(",", ": ")) | ||
| # torchci's authenticated backfill route, which hands the job to the | ||
| # gha-log-uploader lambda. That lambda owns log downloading now; re-implementing | ||
| # it here is how this script and github-status-test used to drift apart. | ||
| HUD_URL = os.environ.get("HUD_URL", "https://hud.pytorch.org") | ||
| LOG_UPLOADER_BOT_KEY = os.environ.get("LOG_UPLOADER_BOT_KEY", "") | ||
|
|
||
|
|
||
| def upload_log( | ||
| client: Octokit, owner: str, repo: str, job_id: int, conclusion: str | ||
| ) -> None: | ||
| # This logic is copied from github-status-test lambda function | ||
| log = client.actions.download_job_logs_for_workflow_run( | ||
| owner=owner, repo=repo, job_id=job_id | ||
| ).json | ||
| def upload_log(owner: str, repo: str, job_id: int, conclusion: str) -> None: | ||
| if not LOG_UPLOADER_BOT_KEY: | ||
| warn("LOG_UPLOADER_BOT_KEY is not set, skipping the log upload...") | ||
| return | ||
|
|
||
| log_path = f"log/{job_id}" | ||
| if repo != "pytorch": | ||
| log_path = f"log/{owner}/{repo}/{job_id}" | ||
| print(f"..Requesting a log upload for {owner}/{repo} job {job_id}") | ||
| request = Request( | ||
| f"{HUD_URL}/api/log-uploader/backfill", | ||
| method="POST", | ||
| data=json.dumps( | ||
| { | ||
| "repo": f"{owner}/{repo}", | ||
| "job_id": job_id, | ||
| "conclusion": conclusion, | ||
| } | ||
| ).encode(), | ||
| headers={ | ||
| "Content-Type": "application/json", | ||
| "Authorization": LOG_UPLOADER_BOT_KEY, | ||
| }, | ||
| ) | ||
|
|
||
| print(f"..Uploading log to {log_path}") | ||
| try: | ||
| # This needs to be in try catch because GitHub doesn't keep log older than 60 days I think | ||
| S3.Object(BUCKET_NAME, log_path).put( | ||
| Body=gzip.compress(log.encode(encoding="UTF-8")), | ||
| ContentType="text/plain", | ||
| ContentEncoding="gzip", | ||
| Metadata={"conclusion": conclusion}, | ||
| ) | ||
|
|
||
| # Invoke log classifier | ||
| urlopen( | ||
| f"https://vwg52br27lx5oymv4ouejwf4re0akoeg.lambda-url.us-east-1.on.aws/?job_id={job_id}&repo={owner}/{repo}" | ||
| ) | ||
| except Exception as error: | ||
| # GitHub drops logs after around 60 days, so an old job simply has | ||
| # nothing to fetch. That is a warning, not a reason to stop the backfill. | ||
| with urlopen(request, timeout=30) as response: | ||
| if response.status != 200: | ||
| warn(f"Log upload for job {job_id} returned {response.status}") | ||
| except (HTTPError, OSError) as error: | ||
| warn( | ||
| f"Failed to upload {log} for job {job_id} from repo {owner}/{repo}: " | ||
| + f"{error}, skipping..." | ||
| f"Failed to request a log upload for job {job_id} from repo " | ||
| f"{owner}/{repo}: {error}, skipping..." | ||
| ) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. comment consistency nit. ⚪ The comment above the `try` block describes a failure this block cannot see. (ai-generated section)A 200 from Reviewed by codex gpt-5.6-sol at xhigh effort, against 5a2abf5. |
||
|
|
||
|
|
||
| def process_event(owner: str, repo: str, event: str, body: Any) -> None: | ||
| # This logic is copied from github-status-test lambda function | ||
| if repo == "pytorch": | ||
| repo_prefix = "" | ||
| else: | ||
| repo_prefix = f"{owner}/{repo}/" | ||
|
|
||
| # Only DynamoDB, which is what clickhouse-replicator-dynamo reads. The raw | ||
| # event archive github-status-test used to write to S3 was never read by | ||
| # anything and is not reproduced here. | ||
| if "id" not in body: | ||
| warn(f"Missing ID in {body}, skipping...") | ||
| return | ||
|
|
||
| id = body["id"] | ||
| print(f"{event}/{repo_prefix}{id}") | ||
| S3.Object(BUCKET_NAME, f"{event}/{repo_prefix}{id}").put( | ||
| Body=json_dumps(body), ContentType="application/json" | ||
| ) | ||
| print(f"{event} {owner}/{repo}/{id}") | ||
|
|
||
| dynamodb_table = "" | ||
| if event == "workflow_run": | ||
|
|
@@ -131,7 +124,7 @@ def process_workflow_run( | |
| conclusion = workflow_job["conclusion"] | ||
|
|
||
| process_event(owner, repo, "workflow_job", workflow_job) | ||
| upload_log(client, owner, repo, job_id, conclusion) | ||
| upload_log(owner, repo, job_id, conclusion) | ||
|
|
||
| if not count or count >= total_count: | ||
| # Finish processing all events | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's make failure explicit? or validate the key presence early
🟡 Without `LOG_UPLOADER_BOT_KEY` the script skips every log upload without failing. (ai-generated section)
upload_logwarns and returns for every job when the key is empty, and it is called once per job fromprocess_workflow_run. Under Python's default warning filter that identical warning is emitted only once per process, so a run over thousands of jobs prints it a single time, early, among the repeated..Processing N jobs...output, then keeps writing DynamoDB rows; an otherwise successful run exits 0 with no log uploaded. A key that is set but wrong is louder — the route answers 403 and the warning carries the job id, so it repeats per job — but that run also ends successfully with nothing uploaded. Validating the key once inbackfill()before the loop starts, and stopping on the first 401 or 403, puts both decisions where the operator can still act on them.Reviewed by codex gpt-5.6-sol at xhigh effort, against 5a2abf5.