Skip to content

chore(site): replace agent log service #9814

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

Merged
merged 5 commits into from
Sep 22, 2023

Conversation

BrunoQuaresma
Copy link
Collaborator

No description provided.

@BrunoQuaresma BrunoQuaresma requested review from kylecarbs and a team September 21, 2023 18:18
@BrunoQuaresma BrunoQuaresma self-assigned this Sep 21, 2023
@BrunoQuaresma BrunoQuaresma requested review from Parkreiner and removed request for a team September 21, 2023 18:18

socket.current = API.watchWorkspaceAgentLogs(agentId, {
// Get all logs
after: 0,
Copy link
Member

Choose a reason for hiding this comment

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

We should not be using after: 0 all the time.

We intentionally have an endpoint to fetch all logs, and then we should stream afterward, just like before. This hopefully would reduce lagging/flickering on the UI as logs stream in from the WS, because they'd be delivered in a payload first.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hm.. I found having this way faster and better since we don't have to do two operations. It is also easy to understand. In this case, there is no lagging or flickering since when using after 0, all the "past" logs are being sent at once and not one by one.

Copy link
Member

@Parkreiner Parkreiner left a comment

Choose a reason for hiding this comment

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

Pinging @kylecarbs in case he still feels strongly about the after property, but everything else looked good to me

@BrunoQuaresma BrunoQuaresma merged commit 85ab9c2 into main Sep 22, 2023
@BrunoQuaresma BrunoQuaresma deleted the bq/replace-agent-logs-service branch September 22, 2023 19:13
@github-actions github-actions bot locked and limited conversation to collaborators Sep 22, 2023
@BrunoQuaresma
Copy link
Collaborator Author

Related to #9598

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants