Update the architecture doc for the log upload path - #8597
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
## What Rewrites the "Logs and log classifications" section to describe the new path (bot handler -> gha-log-uploader -> S3 -> call-log-classifier -> log_classifier) and notes that the raw webhook payload archive goes away with `github-status-test`. ## Why The old text described logs as arriving through "a manual connection with the `pytorch/pytorch` repo", and carried a note saying the logic "could be moved to the `hud.pytorch.org` webhooks endpoint [...] Add suo as a reviewer on a PR if you want to add it!" -- which is what these commits do. ghstack-source-id: 4d9a6ad Pull-Request: #8597
meta-cla
Bot
added
the
CLA Signed
izaitsevfb
left a comment
izaitsevfb
left a comment
There was a problem hiding this comment.
lgtm, but check the discrepancies
| 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]). |
There was a problem hiding this comment.
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.
| 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. |
There was a problem hiding this comment.
is the list intended in the final version or should the doc be updated?
pytorch,meta-pytorch,malfetandvllm-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.
2 participants
Stack from ghstack (oldest at bottom):
What
Rewrites the "Logs and log classifications" section to describe the new path
(bot handler -> gha-log-uploader -> S3 -> call-log-classifier -> log_classifier)
and notes that the raw webhook payload archive goes away with
github-status-test.