Skip to content
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

Merged
merged 1 commit into from
Jan 20, 2022
Merged

Conversation

humphd
Copy link
Contributor

@humphd humphd commented Jan 13, 2022

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.

@humphd humphd added area: autodeployment Anything related to auto deployment area: dashboard Related to Telescope's dashboard (the page that has stats) labels Jan 13, 2022
@humphd humphd self-assigned this Jan 13, 2022
@gitpod-io
Copy link

gitpod-io bot commented Jan 13, 2022

Andrewnt219
Andrewnt219 previously approved these changes Jan 14, 2022
Copy link
Contributor

@Andrewnt219 Andrewnt219 left a 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.

src/api/status/public/js/build-log/check-for-build.js Outdated Show resolved Hide resolved
Copy link
Contributor

@DukeManh DukeManh left a 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.

Andrewnt219
Andrewnt219 previously approved these changes Jan 14, 2022
Andrewnt219
Andrewnt219 previously approved these changes Jan 15, 2022
tpmai22
tpmai22 previously approved these changes Jan 18, 2022
Copy link
Contributor

@tpmai22 tpmai22 left a 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.

Andrewnt219
Andrewnt219 previously approved these changes Jan 19, 2022
Copy link
Contributor

@Andrewnt219 Andrewnt219 left a comment

Choose a reason for hiding this comment

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

LGTM🤞.

DukeManh
DukeManh previously approved these changes Jan 19, 2022
@humphd humphd marked this pull request as ready for review January 19, 2022 13:39
@humphd
Copy link
Contributor Author

humphd commented Jan 19, 2022

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?

@manekenpix
Copy link
Member

@humphd I'll set up the autodeployment server later today (at 6ish) and launch telescope using this branch for testing.

@humphd
Copy link
Contributor Author

humphd commented Jan 20, 2022

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).

@humphd
Copy link
Contributor Author

humphd commented Jan 20, 2022

I missed a lock file fix that went in with @dbelokon's video changes. Rebased and updated.

@humphd humphd merged commit 926b118 into Seneca-CDOT:master Jan 20, 2022
@humphd humphd added this to the 2.5 Release milestone Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: autodeployment Anything related to auto deployment area: dashboard Related to Telescope's dashboard (the page that has stats)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cache build log for current/previous build
6 participants