-
Notifications
You must be signed in to change notification settings - Fork 887
fix!: remove startup logs eof for streaming #8528
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
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.
I think this is fine but I can give it a proper look tomorrow.
if state.ReadyAt.Valid { | ||
return | ||
} | ||
// Just keep listening - more logs might come in the future! |
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.
We can remove the ready state stuff higher up too.
@mafredri this can just wait until Monday, no biggie. |
We have external utilities like logstream-kube that may send logs after an agent shuts down unexpectedly to report additional information. In a recent change we stopped accepting these logs, which broke these utilities. In the future we'll rename startup logs to agent logs or something more generalized so this is less confusing in the future.
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 just remembered the current log fetching in cli/cliui.Agent
relies on the websocket closing when streaming startup logs.
I pushed a fix for this and as long as the WebUI doesn't depend on the ws closing behavior, I think we're good.
Marked this as breaking since this will break coder server/clients (e.g. older clients will continue streaming startup logs forever, when blocking startup script is used). |
We have external utilities like logstream-kube that may send logs after an agent shuts down unexpectedly to report additional information. In a recent change we stopped accepting these logs, which broke these utilities.
In the future we'll rename startup logs to agent logs or something more generalized so this is less confusing in the future.