Update WatcherDelegatingHandler to stop eating first line (#183) #190
Conversation
|
Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA. It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
k8s-ci-robot
added
cncf-cla: no
hossambarakat
force-pushed
the
fix-eating-first-line
branch
from
e9ea7a9 to
ba12b6a
Compare
k8s-ci-robot
added
cncf-cla: yes
| @@ -64,6 +66,37 @@ protected override bool TryComputeLength(out long length) | |||
| length = 0; | |||
| return false; | |||
| } | |||
| } | |||
| internal class PeekableStreamReader : StreamReader | |||
There was a problem hiding this comment.
I'm worried that if someone calls any of the other ReadBlock, Read etc methods this is going to break...
At the very least let's override those messages and have them throw an exception so that we detect that it's happening...
There was a problem hiding this comment.
Makes sense to me. I have updated the pull request to override all the remaining Read methods to throw NotImplementedException
There was a problem hiding this comment.
@brendanburns The build is failing but I have no clue why and I don't have permission to re-trigger the build. Would you please point me to what should I check?
There was a problem hiding this comment.
I managed to re-trigger the build and all pass now.
There was a problem hiding this comment.
@brendanburns, Was wondering if you have time to review the updated request
|
Thanks for the PR! Generally looks good, but one concern. |
…haviour (kubernetes-client#183) Override Read methods in PeekableStreamReader to avoid unpredicted behaviour (kubernetes-client#183)
hossambarakat
force-pushed
the
fix-eating-first-line
branch
from
6d504a0 to
880080a
Compare
|
LGTM, thanks! |
…-client#183) (kubernetes-client#190) * Update WatcherDelegatingHandler to stop eating first line (kubernetes-client#183) * Override Read methods in PeekableStreamReader to avoid unpredicted behaviour (kubernetes-client#183) Override Read methods in PeekableStreamReader to avoid unpredicted behaviour (kubernetes-client#183)
No description provided.