[
https://issues.apache.org/jira/browse/HADOOP-7910?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13176104#comment-13176104
]
Harsh J commented on HADOOP-7910:
---------------------------------
Thanks Sho. The patch looks good. I'll commit it once the following couple of
nits are addressed:
- {{fail("");}} messages such as {{"Too large number"}} is not very
descriptive. These calls will fail the test with the string as the reason, so
something like "Test passed for a number too large" or "Test passed for a
number too small" are more easier to understand when such a thing happens.
Makes sense? (You have the other fail() message correctly written, so just
these last two that relate to Longs).
- The {{IllegalArgumentException}} that carries the message {{"binary prefix is
allowed only k, m, g, t, p, e(case insensitive)"}} can be improved. Perhaps
something more like: {{"Invalid size prefix %char in given string %string.
Allowed prefixes are <set>"}}. Know that exception messages and log messages
cater to users, and if we can be very clear at what's being given to them, it
makes their life easier in hunting down the trouble and fixing it up themselves
:)
- Javadoc for the getLongBytes method can carry a 'case insensitive' comment,
for the devs.
> add configuration methods to handle human readable size values
> --------------------------------------------------------------
>
> Key: HADOOP-7910
> URL: https://issues.apache.org/jira/browse/HADOOP-7910
> Project: Hadoop Common
> Issue Type: Improvement
> Components: conf
> Reporter: Sho Shimauchi
> Assignee: Sho Shimauchi
> Priority: Minor
> Attachments: HADOOP-7910.patch, HADOOP-7910.patch,
> HADOOP-7910.patch.3, hadoop-7910.txt
>
>
> It's better to have a new configuration methods which handle human readable
> size values.
> For example, see HDFS-1314.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
