Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Accumulo, mail # dev - PORT Property Type description does not match regular expression.


Copy link to this message
-
Re: PORT Property Type description does not match regular expression.
Christopher Tubbs 2012-09-09, 05:51
Sorry, that was long. A shorter response:

It should do better than that, but it was only intended as a quick
sanity check, deferring more robust checks to the code that uses the
property.

On Sun, Sep 9, 2012 at 1:49 AM, Christopher Tubbs <[EMAIL PROTECTED]> wrote:
> The regular expression was originally just a simple, quick sanity
> check, not a full-fledged validation. It should probably check for
> range validity, either in the regular expression or in the
> isValidFormat implementation. What I'd like to do with that is replace
> it with a more generic validator. So, instead of just passing in a
> regex to validate, it would pass in a function.
>
> One reason why just a quick and dirty check was done was because every
> property has a separate range of validity, and we can only generalize
> so much by validating the type outside of the context of a particular
> property. The intent was to do a dirty check on the type for sanity,
> then fail more robustly when the property actually needed to be used.
>
> My only concern with such a change (getting rid of regexes for more
> robust validators), is that I don't remember whether the regex showed
> up in the config documentation during the build... if it does, the
> regex still has some utility for documentation.
>
> At the very least, that's a horrible regex for PORT, and I should
> apologize for being so lazy. A better one have at least been
> "\\d{4,5}". Better yet, it should be something like
> "102[4-9]|10[3-9]\\d|1[1-9]\\d{2}|[2-9]\\d{3}|[1-5]\\d{4}|6[0-4]\\d{3}|65[0-4]\\d{2}|655[0-2]\\d|6553[0-5]"
> (untested against java-specific syntax, but I think it is complete in
> principle).
>
> If the regex is being shown on any generated configuration
> documentation still, this longer one wouldn't be very useful to read.
> I still like the validator concept to replace the regex. Descriptive
> sentences are probably best for documentation, anyway, not regexes.
>
> If you create a ticket, feel free to assign it to me. I can create a
> patch for 1.5.0 to improve the validation. I had some other ideas
> about improving config anyway (possible discussions to follow).
>
> On Sat, Sep 8, 2012 at 9:55 PM, David Medinets <[EMAIL PROTECTED]> wrote:
>> PropertyType
>>
>>   PORT("port", "\\d{1,5}", "An positive integer in the range
>> 1024-65535, not already in use or specified elsewhere in the
>> configuration"),
>>
>> I was writing some unit tests for PropertyType and ran across PORT:
>>
>>         case PORT:
>>           assertFalse(pt.name(), pt.isValidFormat(null));
>>           assertFalse(pt.name(), pt.isValidFormat("1023"));
>>           for (int i = 1024; i < 65536; i++) {
>>             assertTrue(pt.name(), pt.isValidFormat(new Integer(i).toString()));
>>           }
>>           assertFalse(pt.name(), pt.isValidFormat("65536"));
>>
>> The second test failed. And, of course, when I checked the regular
>> expression it allows any number from "0" to "99999". Should the
>> isValidFormat method actually test the port range?