The changes are available in GitHub, I just overlooked it. And they’re actually neatly contained in two commits:
The changes to storm-kafka can be found here: https://github.com/jwplayer/storm/commit/2069c76695a225e4bb8f402c89e572836104755a <https://github.com/jwplayer/storm/commit/2069c76695a225e4bb8f402c89e572836104755a> The changes to storm-redis are here: https://github.com/jwplayer/storm/commit/30d000d3ff673efa8b927d23e554a705fb2928b8 <https://github.com/jwplayer/storm/commit/2069c76695a225e4bb8f402c89e572836104755a> The changes to storm-kafka are straightforward and implemented in such a way that they would be useful for use cases outside of SQE. As the commit message states, it adds a new kafka deserialization scheme (FullScheme) that includes the key, value, topic, partition and offset when reading from kafka, which is a feature I can see as being valuable for some use cases. I would be +1 for merging that code. The changes to storm-redis are a little different, as Morrigan pointed out, because it only addresses the Trident API, but IMHO it looks like a good direction. @HeartSavior — Would you have some time to take a look at the storm-redis changes and provide your opinion, since you’re one of the original authors of that code? -Taylor > On Sep 26, 2016, at 6:28 PM, Jungtaek Lim <kabh...@gmail.com> wrote: > > Great! > > For storm-redis, we might need to modify key/value mapper to use byte[] > rather than String. > When I co-authored storm-redis, I forgot considering binary format of > serde. If we want to address that part, we can also address it. > > 2016년 9월 27일 (화) 오전 7:19, Morrigan Jones <morri...@jwplayer.com>님이 작성: > >> Sure, when I can. Storm-kafka should be pretty easy. The storm-redis one >> will require more work to make it more complete. >> >> On Mon, Sep 26, 2016 at 6:09 PM, P. Taylor Goetz <ptgo...@gmail.com> >> wrote: >> >>> Thanks for the explanation Morrigan! >>> >>> Would you be willing to provide a pull request or patch so the community >>> can review? >>> >>> It sounds like at least some of the changes you mention could be useful >> to >>> the broader community (beyond the SQL effort). >>> >>> Thanks again, >>> >>> -Taylor >>> >>>> On Sep 26, 2016, at 4:40 PM, Morrigan Jones <morri...@jwplayer.com> >>> wrote: >>>> >>>> storm-kafka - This is needed because storm-kafka does not provide a >>> scheme >>>> class that gives you the key, value (payload), partition, and offset. >>>> MessageMetadataScheme.java comes comes closest, but is missing the key. >>>> This was a pretty simple change on my part. >>>> >>>> storm-redis - This is needed for proper support of Redis hashes. The >>>> existing storm-redis uses a static string (additionalKey in >>>> the RedisDataTypeDescription class) for the field name in hash types. I >>>> updated it to use a configurable KeyFactory for both the hash name and >>> the >>>> field name. We also added some limited support for set types. This is >>>> admittedly the messiest between the two jars since we only cared about >>> the >>>> trident states and would require a lot more changes to get storm-redis >>> more >>>> "feature complete" overall. >>>> >>>> >>>> >>>> >>>>> On Mon, Sep 26, 2016 at 4:03 PM, P. Taylor Goetz <ptgo...@gmail.com> >>> wrote: >>>>> >>>>> Sounds good. I’ll find out if it builds against 2.x. If so I’ll go >> that >>>>> direction. Otherwise I’ll come back with my findings and we can >> discuss >>> it >>>>> further. >>>>> >>>>> I notice there are jars in the git repo that we obviously can’t >> import. >>>>> They look like they might be custom JWPlayer builds of storm-kafka and >>>>> storm-redis. >>>>> >>>>> Morrigan — Do you know if there is any differences there that required >>>>> custom builds of those components? >>>>> >>>>> -Taylor >>>>> >>>>>>> On Sep 26, 2016, at 3:31 PM, Bobby Evans >> <ev...@yahoo-inc.com.INVALID >>>> >>>>>> wrote: >>>>>> >>>>>> Does it compile against 2.X? If so I would prefer to have it go >> there, >>>>> and then possibly 1.x if people what it there too. - Bobby >>>>>> >>>>>>> On Monday, September 26, 2016 12:47 PM, P. Taylor Goetz < >>>>>> ptgo...@gmail.com> wrote: >>>>>> >>>>>> >>>>>> The IP Clearance vote has passed and we are now able to import the >> SQE >>>>> code. >>>>>> >>>>>> The question now is to where do we want to import the code? >>>>>> >>>>>> My inclination is to import it to “external” in the 1.x branch. It >> can >>>>> be ported to other branches as necessary/if desired. An alternative >>> would >>>>> be to treat it as a feature branch, but I’d rather take the former >>> approach. >>>>>> >>>>>> Thought/opinions? >>>>>> >>>>>> -Taylor >>>>>> >>>>>>> On Sep 21, 2016, at 8:39 PM, P. Taylor Goetz <ptgo...@gmail.com> >>> wrote: >>>>>>> >>>>>>> My apologies. I meant to cc dev@, but didn't. Will forward in a >>> bit... >>>>>>> >>>>>>> The vote (lazy consensus) is underway on general@incubator, and >> will >>>>> close in less than 72 hours. After that the code can be merged. >>>>>>> >>>>>>> -Taylor >>>>>>> >>>>>>>> On Sep 21, 2016, at 7:02 PM, Jungtaek Lim <kabh...@gmail.com> >> wrote: >>>>>>>> >>>>>>>> Hi dev, >>>>>>>> >>>>>>>> While code contribution of SQE is in progress, I would like to >>> continue >>>>>>>> discussion how to merge SQE and Storm SQL. >>>>>>>> >>>>>>>> I did an analysis of merging SQE and Storm SQL in both side, >>>>> integrating >>>>>>>> SQE to Storm SQL and vice versa. >>>>>>>> https://cwiki.apache.org/confluence/display/STORM/ >>>>> Technical+analysis+of+merging+SQE+and+Storm+SQL >>>>>>>> >>>>>>>> As I commented to that page, since I'm working on Storm SQL for >> some >>>>> weeks >>>>>>>> I can be (heavily) biased. So I'd really appreciated if someone can >>> do >>>>>>>> another analysis. >>>>>>>> >>>>>>>> Please feel free to share your thought about this analysis, or >>> another >>>>>>>> proposal if you have any, or other things about the merging. >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Jungtaek Lim (HeartSaVioR) >>>> >>>> >>>> -- >>>> Morrigan Jones >>>> Principal Engineer >>>> *JW*PLAYER | Your Way to Play >>>> morri...@jwplayer.com | jwplayer.com >>> >> >> >> >> -- >> Morrigan Jones >> Principal Engineer >> *JW*PLAYER | Your Way to Play >> morri...@jwplayer.com | jwplayer.com >>
signature.asc
Description: Message signed with OpenPGP using GPGMail
