Home | About | Sematext search-lucene.com search-hadoop.com
NEW: Monitor These Apps!
elasticsearch, apache solr, apache hbase, hadoop, redis, casssandra, amazon cloudwatch, mysql, memcached, apache kafka, apache zookeeper, apache storm, ubuntu, centOS, red hat, debian, puppet labs, java, senseiDB
 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.
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?
NEW: Monitor These Apps!
elasticsearch, apache solr, apache hbase, hadoop, redis, casssandra, amazon cloudwatch, mysql, memcached, apache kafka, apache zookeeper, apache storm, ubuntu, centOS, red hat, debian, puppet labs, java, senseiDB