-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Fix remote logs #7660
Fix remote logs #7660
Conversation
Docker compatibility - logs endpoint does not write stream headers if container has a tty Signed-off-by: Ashley Cui <[email protected]>
@vrothberg tagging so you have the link to this PR, thanks for helping me on this |
Update: this should fix remote logs the best we can: adding a tty to a container makes the logs add return carriage symbols, which is a tty quirk, so this should be an accpetable fix. See: moby/moby#37366 (comment) |
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 |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ashley-cui, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/hold cancel |
Docker compatibility - logs endpoint does not write stream headers if container has a tty
There are still some issues with logging behavior that affects this, we need to sort that out in the backend before we can know for certain if this is the right fix
Fixes: #7196
Signed-off-by: Ashley Cui [email protected]