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

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


+
David Medinets 2012-09-09, 01:55
Copy link to this message
-
Re: PORT Property Type description does not match regular expression.
Christopher Tubbs 2012-09-09, 05:49
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?
+
Christopher Tubbs 2012-09-09, 05:51