accumulo-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Josh Elser <josh.el...@gmail.com>
Subject Re: ACCUMULO-1604 Bug Fix
Date Tue, 05 Jul 2016 16:23:08 GMT
Feel free to use `git-am` to create a patch or just submit a 
pull-request against the master branch of apache/accumulo on Github.

Mike Miller wrote:
> 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<keith@deenlo.com>  wrote:
>
>> On Fri, Jul 1, 2016 at 3:24 PM, Mike Miller<michaelpmiller@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<keith@deenlo.com>  wrote:
>>>
>>>> On Fri, Jul 1, 2016 at 1:29 PM, Mike Miller<michaelpmiller@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.
>>>>>
>

Mime
View raw message