Skip to content

Add the log uploader bot - #8594

Open
huydhn wants to merge 11 commits into
gh/huydhn/5/basefrom
gh/huydhn/5/head
Open

Add the log uploader bot#8594
huydhn wants to merge 11 commits into
gh/huydhn/5/basefrom
gh/huydhn/5/head

Conversation

@huydhn

@huydhn huydhn commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Stack from ghstack (oldest at bottom):

Impact: none until LOG_UPLOADER_REPOS is set -- an unset allowlist
disables the handler
Risk: low

What

A new Probot handler on workflow_job that, when a job completes, asks the
gha-log-uploader lambda to archive its log. Adds lib/lambda.ts, which wraps
the async invoke, and adds the AWS SDK client-lambda package.

Which repos are enabled is controlled by LOG_UPLOADER_REPOS, a comma-separated
list of owner/repo or owner/*. Unset means the handler does nothing.

[ghstack-poisoned]
@vercel

vercel Bot commented Aug 20, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
torchci Ignored Ignored Aug 21, 2026 10:51pm

Request Review

huydhn added 2 commits August 20, 2026 13:01
[ghstack-poisoned]
[ghstack-poisoned]
huydhn added 3 commits August 20, 2026 15:17
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
@huydhn
huydhn marked this pull request as ready for review August 20, 2026 23:55
@huydhn
huydhn requested a review from izaitsevfb August 20, 2026 23:55
huydhn added 4 commits August 20, 2026 23:41
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]
[ghstack-poisoned]

@izaitsevfb izaitsevfb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

Comment thread torchci/package.json
},
"dependencies": {
"@aws-sdk/client-dynamodb": "^3.347.1",
"@aws-sdk/client-lambda": "^3.347.1",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably not intentional?


⚪ The new `@aws-sdk/client-lambda` resolves to 3.1114.0, while every other AWS SDK package in this lockfile is 3.347.1 or older. (ai-generated section)

torchci/package.json asks for ^3.347.1, the same range as the S3 and DynamoDB clients beside it. Those two are already locked at 3.347.x, but nothing had resolved this package before, so the caret took the newest release available.

The result is that a modern SDK tree now sits beside the old one in yarn.lock, with nine of the same package names now present at two versions each: the whole @aws-sdk/credential-provider-* set, @aws-sdk/token-providers, @aws-sdk/types and @smithy/types each appear once at 3.347.0 / 1.0.0 and once at 3.97x / 4.17.2. The credential-provider chain that comes with it is never exercised, since this code passes its credentials explicitly.

If the jump was not deliberate, resolving this entry to the version its siblings already use would keep one tree instead of two.

Reviewed by codex gpt-5.6-sol at xhigh effort, against a3bbcc4.

Comment thread torchci/lib/lambda.ts
// also has to fit inside.
maxAttempts: 2,
requestHandler: { connectionTimeout: 1000, requestTimeout: 2000 },
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's make sure that permission exists


Does the identity behind `OUR_AWS_ACCESS_KEY_ID` already have `lambda:InvokeFunction` on `gha-log-uploader`? (ai-generated section)

The client authenticates with the OUR_AWS_* pair torchci already uses for S3 and DynamoDB, and the lambda at the base of this stack documents lambda:InvokeFunction as its only way in. No grant of it appears anywhere in this stack, which may just mean it lives outside this repository.

If it is missing, the failure is quiet: every invoke returns access-denied, the handler catches it, and GitHub still gets its acknowledgement. Nothing else in this code notices, so once a repo has no old webhook behind it its logs would simply stop arriving. Worth confirming before the first repo goes into LOG_UPLOADER_REPOS.

Reviewed by codex gpt-5.6-sol at xhigh effort, against a3bbcc4.

@huydhn huydhn Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can't atm, I need to add it later

[ghstack-poisoned]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants