[ 
https://issues.apache.org/jira/browse/HADOOP-15056?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16280817#comment-16280817
 ] 

Jack Bearden commented on HADOOP-15056:
---------------------------------------

Really great feedback, thanks for reviewing the code [~xiaochen]. I really 
appreciate that you took the time to give me specific feedback on how I could 
improve. My responses are below:

{quote}Can we modify the test to something like this? [code removed]{quote}
That test snippet you provided was actually really helpful and I implemented it 
in patch #5 as best I could.

{quote}About the message, while it good to have this information when a stream 
doesn't support unbuffer, WARN may be intrusive for some cases. For example, I 
think it's not unreasonable for an application to write some general codes to 
unbuffer regardless, but after this they'll see WARN. It's true this is the 
only log for that logger, but they still will catch this and have to change the 
log level. DEBUG may be a better option, where people can opt-in.{quote}
You are totally right. I was thinking WARN was it's own log level for some 
reason -- DEBUG is the appropriate call here. I have modified the log statement 
in patch #5.

{quote}Can we change the text of the UnsupportedOperationException? Maybe 
something like claims to have unbuffer capabilty but does not to implement 
CanUnbuffer ?{quote}
I have updated the error message.


 [~jzhuge] [~xiaochen]  Please review and let me know if anything was missed. 
Thank you again

> Fix TestUnbuffer#testUnbufferException failure
> ----------------------------------------------
>
>                 Key: HADOOP-15056
>                 URL: https://issues.apache.org/jira/browse/HADOOP-15056
>             Project: Hadoop Common
>          Issue Type: Improvement
>    Affects Versions: 2.9.0
>            Reporter: Jack Bearden
>            Assignee: Jack Bearden
>            Priority: Minor
>         Attachments: HADOOP-15056.001.patch, HADOOP-15056.002.patch, 
> HADOOP-15056.003.patch, HADOOP-15056.004.patch, HADOOP-15056.005.patch
>
>
> Hello! I am a new contributor and actually contributing to open source for 
> the very first time. :) 
> I pulled down Hadoop today and when running the tests I encountered a failure 
> with the TestUnbuffer#testUnbufferException test.
> The unbuffer code has recently gone through some changes and I believe this 
> test case may have been overlooked. Using today's git commit 
> (659e85e304d070f9908a96cf6a0e1cbafde6a434), and upon running the test case, 
> there is an expect mock for an exception UnsupportedOperationException that 
> is no longer being thrown. 
> It would appear that a test like this would be valuable so my initial 
> proposed patch did not remove it. Instead, I removed the conditions that were 
> guarding the cast from being able to fire -- as was the previous behavior. 
> Now when we encounter an object that doesn't have the UNBUFFERED 
> StreamCapability, it will throw an error as it did prior to the recent 
> changes. 
> Please review and let me know what you think! :D



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to