Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 23 additions & 11 deletions torchci/docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,17 +63,23 @@ These are uploaded to S3 directly by the GitHub workflows in `pytorch/pytorch`.

### Logs and log classifications

Logs are downloaded by the [`github-status-test`] AWS lambda, which receives
webhooks through a manual connection with the `pytorch/pytorch` repo.
`github-status-test` then pings another lambda, [`log-classifier`], which
performs log classification (more detail in the [README]).
The PyTorch bot's `workflow_job` handler (`lib/bot/logUploader.ts`) asynchronously
invokes the [`gha-log-uploader`] lambda when a job completes. That lambda
downloads the log from GitHub, puts it in the [`ossci-raw-job-status`] bucket
under `log/`, and then asynchronously invokes [`log-classifier`] to do the
classification (more detail in the [README]).

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.

flagging discrepancy.


🟡 The classifier call is described as asynchronous; it is synchronous. (ai-generated section)

gha-log-uploader calls log_classifier through the classifier's Lambda function URL with a blocking urlopen, and function URLs only support the RequestResponse invocation type — so the uploader stays alive until classification finishes. Both the lambda's module docstring and aws/lambda/gha-log-uploader/README.md state this, and both make the 900s function timeout a consequence of it, so as written the architecture doc contradicts the component it describes. Dropping the word, or replacing it with "synchronously, through its function URL", would match. The earlier "asynchronously" in the same paragraph, about invoking the uploader itself, is correct and should stay.


[readme]: https://github.com/pytorch/test-infra/blob/main/aws/lambda/log-classifier/README.md

The logic in `github-status-test` could be moved to the `hud.pytorch.org`
webhooks endpoint, and modified so that all repos that `torchci` is installed
on benefits from log downloads and classifications, but this is not currently
implemented. Add @suo as a reviewer on a PR if you want to add it!
Because this hangs off the App webhook rather than a per-repo one, every repo the
bot is installed on gets log downloads and classifications without an admin
configuring anything. Which repos are enabled is controlled by the
`LOG_UPLOADER_REPOS` env var while the cutover from [`github-status-test`] is in
progress; see https://github.com/pytorch/test-infra/issues/7549.

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.

is the list intended in the final version or should the doc be updated?

pytorch, meta-pytorch, malfet and vllm-project


🟡 `LOG_UPLOADER_REPOS` is not the only gate on which repositories get log uploads. (ai-generated section)

torchci/lib/bot/logUploader.ts returns before invoking the uploader unless isPyTorchbotSupportedOrg(owner) || isVLLM(owner) holds, and torchci/lib/bot/utils.ts accepts only the owners pytorch, meta-pytorch, malfet and vllm-project; only then is LOG_UPLOADER_REPOS consulted. The paragraph presents that env var as the repository selector and frames it as temporary for the cutover, but at this head a repository under any other owner is rejected before the variable is read at all, so "every repo the bot is installed on gets log downloads and classifications without an admin configuring anything" is wider than what the handler does. Naming both gates, or narrowing the sentence to the supported organisations, would fix it.


Missing logs are re-requested through `backfillMissingLog` in `lib/jobUtils.ts`,
which Dr.CI calls when it finds a failed job with no log. Callers outside HUD use
the authenticated `POST /api/log-uploader/backfill` route.

### Test statistics

Expand All @@ -91,11 +97,17 @@ an example.

### Raw webhook payloads

For archival purposes, we save all raw webhook payloads to S3. This is done
through the [`github-status-test`] lambda (the same one that downloads logs).
The raw webhook payloads are uploaded to the [`ossci-raw-job-status`] S3 bucket.
The [`github-status-test`] lambda archives raw webhook payloads to the
[`ossci-raw-job-status`] S3 bucket, under a prefix per event type. Nothing reads
them: `clickhouse-replicator-s3` has no `SUPPORTED_PATHS` entry for
`workflow_job/`, `workflow_run/`, or `full_workflow_*/`, and ClickHouse gets jobs
from DynamoDB through `clickhouse-replicator-dynamo`.

This archive goes away with the lambda. It is not reproduced in
[`gha-log-uploader`].

[`github-status-test`]: https://us-east-1.console.aws.amazon.com/lambda/home?region=us-east-1#/functions/github-status-test?tab=code
[`gha-log-uploader`]: https://github.com/pytorch/test-infra/blob/main/aws/lambda/gha-log-uploader/README.md
[`ossci-raw-job-status`]: https://s3.console.aws.amazon.com/s3/buckets/ossci-raw-job-status?region=us-east-1&tab=overview

## Adding a new repo to `torchci`
Expand Down
Loading