-
Notifications
You must be signed in to change notification settings - Fork 189
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
Cache and display build logs #2653
Conversation
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.
Can't say it works for sure because I cannot run the auto-deployment server, but the logic looks straightforward to me. As I understand, the server now additionally writes to the build.cache
while streaming the new data. And on response, it writes the cache first before appending new data.
Some logic changes in the client but it should work just fine. The code refactoring is a nice addition 👍. Maybe we will need a date library down the road or just wait for Temporal.
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.
Great work 👍 coding in the dark.
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.
I think it's good, I learned a lot.
ea9f471
to
26e309a
Compare
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.
LGTM🤞.
I'm not sure what else to do on this without actually merging it and testing live. @manekenpix I assume you would have to manually set this up if I merged? Is there a time that's good for us to try that, so I don't break staging? |
@humphd I'll set up the autodeployment server later today (at 6ish) and launch telescope using this branch for testing. |
26e309a
to
1ffb8c7
Compare
d690c08
to
0a7b5a7
Compare
Fixed a bunch of crashes @manekenpix found on staging, rebased and integrated the work that landed recently on the build log header. I think this is ready (again). |
0a7b5a7
to
90bb363
Compare
I missed a lock file fix that went in with @dbelokon's video changes. Rebased and updated. |
Fixes #2535.
This adds a cache to the build log stream, so that when new clients connect, we start by sending the beginning of the log that they missed (if any). It also allows us to stream the previous build's log, so we can always display the last build that completed.
The basic idea is that when we create the stream for the build process, we also create a
cache
stream buffer, which we fill with the data from the build output. Then, whenever we need to send that data back again, we can get its contents and return. This also allows us to remember the previous build's log and send it to whoever wants it later on.I have tested nothing in this PR (yolo!) and I'm not sure how to test this without running it on staging. This is just a draft to get things started.