- Notifications
You must be signed in to change notification settings - Fork928
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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 sendlogs after an agent shuts down unexpectedly to report additionalinformation. 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 somethingmore generalized so this is less confusing in the future.
mafredri left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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 incli/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.