feat(cloud): cloud task service & rendering#932
Conversation
joshsny
left a comment
joshsny
left a comment
There was a problem hiding this comment.
noice - left some things to address, @charlesvien is thinking a bit about how to abstract local / cloud runs well in twig, so might be worth talking to him about that but it's not a blocker on getting this baby shipped 🚢
| return { success: !!prUrl, message: prUrl ? "OK" : "No PR found", prUrl }; | ||
| } | ||
|
|
||
| public async getPrChangedFiles(prUrl: string): Promise<ChangedFile[]> { |
There was a problem hiding this comment.
Should we do this based on a PR or based on a branch? We might not always push a PR, but we will always eventually push to the branch from the cloud run
There was a problem hiding this comment.
Great point, basically I went with PR due to the thought of this being a long-ish running task and avoid back and forth of updating the state of changed files in the panel.
It does make more sense on the branch level though, you're right, can do this via compare on each commit push (so we don;t wait for the full-fledge PR opening).
I would probably keep the PR changed files path as ultimate source of truth when we do open a PR, just to avoid possible weird stale states, should be rare but still.
| } | ||
| } | ||
|
|
||
| // Fallback: use session_logs API when presigned URL is missing or returned no data |
There was a problem hiding this comment.
what's the advantage to the presigned url here, can we just use session logs all the time?
There was a problem hiding this comment.
Make sense, refactored into using that.
| requestFocus(taskId); | ||
| }, [taskId, requestFocus]); | ||
|
|
||
| // Cloud session loading — fetch S3 logs and create a read-only session |
There was a problem hiding this comment.
This logic feels very messy, and should ideally be pulled out into a service with a trpc subscription for getting logs / status changes
There was a problem hiding this comment.
Yeah, felt like having something quick to test against Modal but just makes sense for this do be done proper first pass, did some work there. Thanks!
tatoalo
changed the title
chore: better pagination slurp chore: log writer flush chore: cleaner chore: cleaner
tatoalo
force-pushed
the
feat/twig/cloud-task-changes-and-status
branch
from
ebcf7f4 to
16c9b11
Compare
2 participants
Summary
ghwith slurp pagination GitHub.Before:
twig_cloud_run_before.mp4
After:
twig_cloud_run_after.mp4