Yeah totally, all the cleanups should be independent, this thread just reminded me to file tickets for them.
-jay On Sunday, February 8, 2015, Gwen Shapira <gshap...@cloudera.com> wrote: > I think the new tickets can be done in parallel, and are not an actual > dependency for KAFKA-1845. Is that correct? > > On Sat, Feb 7, 2015 at 1:44 PM, Jay Kreps <jay.kr...@gmail.com > <javascript:;>> wrote: > > > I don't think we need a KIP/vote here, this is just an internal > > refactoring. We had said previously and noted in the document that the > KIPs > > were just for big new features or public api changes. > > > > I am a big +1 on the idea. We'll have to be careful in the code review > > since it would really easy to cause subtle issues and it is hard to > review > > this kind of change. > > > > For what it is worth the high-level idea of adding a bunch of helper code > > in org.apache.kafka.common is to start to incorporate this on the server > > and replace the utilities there. This will just help reduce the total > code > > size. > > > > A few of the highlights there are: > > 1. Replace kafka.utils.Utils with o.a.k.common.utils.Utils. This will > > likely involve some thought and refactoring. Anything non-general purpose > > should move out of Utils entirely and anything that remains should be > high > > quality, general purpose, and have some tests. We may want to keep a > > ScalaUtils with a couple of things that aren't really doable/convenient > in > > Java. This should be straight-forward. > > 2. Refactor the network server to make use of the classes in > > o.a.k.common.network (receive, send, etc). It might be doable to make use > > of Selector as well. > > 3. Replace the request classes in kafka.api with the ones in > > o.a.k.common.requests. This is one of the more valuable things we can do > as > > that will get us to having a single definition of the protocol. > > 4. Make use of the exceptions in o.a.k.common.errors > > 5. Switch over to the new metrics library. > > > > I'll file tickets for these. > > > > -Jay > > > > On Fri, Feb 6, 2015 at 11:16 AM, Joe Stein <joe.st...@stealth.ly > <javascript:;>> wrote: > > > > > I created KIP-12 > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-12+change+broker+configuration+properties+to+be+consistent+with+the+rest+of+the+code > > > and linked it to this thread and the JIRA with the v1 patch. The > rebased > > > version with updates for the current review should be ready to review > in > > > the next few days. > > > > > > On Fri, Feb 6, 2015 at 1:31 PM, Jeff Holoman <jholo...@cloudera.com > <javascript:;>> > > > wrote: > > > > > > > I think this is a good change. Is there general agreement that we are > > > > moving forward with this approach? It would be nice to start using > this > > > for > > > > future work. > > > > > > > > Thanks > > > > > > > > Jeff > > > > > > > > On Tue, Feb 3, 2015 at 9:34 AM, Joe Stein <joe.st...@stealth.ly > <javascript:;>> > > wrote: > > > > > > > > > I updated the RB changing some of the HIGH to MEDIUM and LOW. > > > > > > > > > > There might be other or different opinions and they may change over > > > time > > > > so > > > > > I don't really see h/m/l as a blocker to the patch going in. > > > > > > > > > > It would be great to take all the rb feedback from today and then > > > > tomorrow > > > > > rebase and include changes for a new patch. > > > > > > > > > > Then over the next day or two review, test and commit to trunk (or > > > > re-work > > > > > if necessary). > > > > > > > > > > /******************************************* > > > > > Joe Stein > > > > > Founder, Principal Consultant > > > > > Big Data Open Source Security LLC > > > > > http://www.stealth.ly > > > > > Twitter: @allthingshadoop <http://www.twitter.com/allthingshadoop > > > > > > > ********************************************/ > > > > > > > > > > On Tue, Feb 3, 2015 at 4:56 AM, Andrii Biletskyi < > > > > > andrii.bilets...@stealth.ly <javascript:;>> wrote: > > > > > > > > > > > It'd be great to have it on trunk. > > > > > > As I mentioned under jira ticket (KAFKA-1845) current > > implementation > > > > > lacks > > > > > > correct Importance settings. > > > > > > I'd be grateful if somebody could help me with it (a simple > mapping > > > > > between > > > > > > config setting and importance or comments right in the review > board > > > > would > > > > > > suffice). > > > > > > > > > > > > Thanks, > > > > > > Andrii Biletskyi > > > > > > > > > > > > On Mon, Feb 2, 2015 at 11:38 PM, Gwen Shapira < > > gshap...@cloudera.com <javascript:;> > > > > > > > > > > wrote: > > > > > > > > > > > > > Strong +1 from me (obviously). Lots of good reasons to do it: > > > > > > > consistency, code reuse, better validations, etc, etc. > > > > > > > > > > > > > > I had one comment on the patch in RB, but it can also be > > refactored > > > > as > > > > > > > follow up JIRA to avoid blocking everyone who is waiting on > this. > > > > > > > > > > > > > > Gwen > > > > > > > > > > > > > > On Mon, Feb 2, 2015 at 1:31 PM, Joe Stein < > joe.st...@stealth.ly <javascript:;>> > > > > > wrote: > > > > > > > > Hey, I wanted to start a quick convo around some changes on > > > trunk. > > > > > Not > > > > > > > sure > > > > > > > > this requires a KIP since it is kind of internal and > shouldn't > > > > affect > > > > > > > users > > > > > > > > but we can decide if so and link this thread to that KIP if > so > > > (and > > > > > > keep > > > > > > > > the discussion going on the thread if makes sense). > > > > > > > > > > > > > > > > Before making any other broker changes I wanted to see what > > folks > > > > > > thought > > > > > > > > about https://issues.apache.org/jira/browse/KAFKA-1845 > > ConfigDec > > > > > > patch. > > > > > > > > > > > > > > > > I agree it will be nice to standardize and use one > > configuration > > > > and > > > > > > > > validation library across the board. It helps in a lot of > > > different > > > > > > > changes > > > > > > > > we have been discussing also in 0.8.3 and think we should > make > > > sure > > > > > it > > > > > > is > > > > > > > > what we want if so then: review, commit and keep going. > > > > > > > > > > > > > > > > Thoughts? > > > > > > > > > > > > > > > > /******************************************* > > > > > > > > Joe Stein > > > > > > > > Founder, Principal Consultant > > > > > > > > Big Data Open Source Security LLC > > > > > > > > http://www.stealth.ly > > > > > > > > Twitter: @allthingshadoop < > > > http://www.twitter.com/allthingshadoop > > > > > > > > > > > > > ********************************************/ > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > Jeff Holoman > > > > Systems Engineer > > > > > > > > > >