-
Notifications
You must be signed in to change notification settings - Fork 384
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
Added MultiplexedStreamReader #473
base: master
Are you sure you want to change the base?
Conversation
var line = new List<byte>(); | ||
|
||
var buffer = new byte[1]; | ||
|
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.
@The-TT-Hacker I tested this method trying to get rid of headers being shown on MonitorStreamAsync
method and haven't managed to cancel it.
Would you confirm if expected behaviour is to read until 0 bytes are returned or to stop reading when no more lines are retrieved?
When res
equals 0, it loops indefinitelly.
This is test I used
https://gist.github.com/dgvives/2620960049191797ea0d871c339d9bc2#file-program-cs-L1
It would be nice if MultiplexedStreamReader could detect stream being multiplexed and provide output por ReadLineAsync without control chars.
Have you checked this running containers on different platforms? Because of EOL
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.
@ACoderLife hey! Sorry for taking long. I'm cleaning up and merging so PRs with fixes and improvements before I start the wave of changes on the whole codebase.
That being said, have you checked @dgvives concern with the infinite loop?
Thank you!
@galvesribeiro I have updated this one with cancellation support. |
Added a simple reader for the multiplexed stream used for getting logs from containers which can read lines as strings. Also works with the service logs pull request i've made.
Ideally the multiplex stream would extend the stream class or something so we can use the normal stream reader but this is working for me.