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.
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?
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