OK since the problem is really with the ColumnAgeOffFilter, I will go with the simple check in the constructor and not make any changes to Filter. Is there a preferred technique of code submission for non-committers?
On Fri, Jul 1, 2016 at 3:28 PM, Keith Turner <ke...@deenlo.com> wrote: > On Fri, Jul 1, 2016 at 3:24 PM, Mike Miller <michaelpmil...@gmail.com> > wrote: > > > What about an enum in the superclass (Filter.java) that contains all the > > options specific to Filter? > > public static enum FilterOptions { NEGATE } > > And then skip any options in that enum in the constructor of TTLSet. > > Someone would still have to include new options in the enum but at least > it > > would be more obvious. > > > > > That seems ok. Could also have a protected method, something like > super.isValidOption(String). Not sure I like that method name. A method > gives the super class more flexibility. > > > > > > On Fri, Jul 1, 2016 at 2:32 PM, Keith Turner <ke...@deenlo.com> wrote: > > > > > On Fri, Jul 1, 2016 at 1:29 PM, Mike Miller <michaelpmil...@gmail.com> > > > wrote: > > > > > > > Hello Devs, > > > > > > > > For those of you I have yet to meet, my name is Mike Miller and I am > > the > > > > newbie who is going to be helping out on Accumulo development. I have > > > been > > > > working with Accumulo as a Java developer on an ingest and query > > project > > > > for the past year. I doubt I'll be able to fill the shoes of past > > > > contributors but I hope that I can make a positive contribution to > the > > > > project. > > > > > > > > I have browsed the JIRA tickets labelled "newbie" and selected the > > > > ACCUMULO-1604 <https://issues.apache.org/jira/browse/ACCUMULO-1604> > > bug > > > to > > > > fix (more so as a learning exercise than anything). I just wanted to > > make > > > > sure I am reproducing the bug correctly and to propose a solution. > > > > > > > > I believe I reproduced the bug by getting the ColumnAgeOffFilter to > > throw > > > > an "IllegalArgumentException: bad TTL options" in the > > > > o.a.a.core.iterators.user.FilterTest.test2a(). I did this by adding > the > > > > following line: > > > > ColumnAgeOffFilter.setNegate(is, true); > > > > This Exception is caused by: NumberFormatException: For input string > > > "true" > > > > > > > > The comments in the ticket discuss adding "column:" prefix to options > > for > > > > the ColumnAgeOffFilter. Please correct me if I am wrong but I think > it > > > can > > > > be fixed without having to change the syntax of the options. Wouldn't > > > > adding a check to the loop in the constructor of TTLSet to ignore the > > > > NEGATE option on any strings in objectStrings param prevent this > error > > > from > > > > occurring? The negate option would still be seen by the parent, > > without > > > > any further changes. > > > > > > > > > > > > > That would fix it. It does not solve the case where new options are > > added > > > to the superclass in the future. However the option you proposed is > > > infinitely better than doing nothing and I would be in favor of making > > that > > > change. > > > > > > Implementing the "column:" prefix in a backwards compatible way is very > > > tricky, especially if considering the case where someone currently has > an > > > actual column name with that prefix. > > > > > > > > > > > > > > Looking forward to working with you folks! > > > > -Mike M. > > > > > > > > > >