Indeed it does not compile against any version due to the missing changes to storm-kafka and storm-redis that Morrigan pointed out.
So for now I’m leaning toward a feature branch so we can at least import the code as is (knowing it will not compile). From there we can request pull requests from the JWPlayer developers for the pieces missing from storm-kafka and storm-redis, and evaluate pulling those changes into other branches. I’d like to propose creating a “sqe_import” branch based on master, and importing the SQE code into a “storm-sqe” module in external. I would incorporate it into the main build, but make it the last module to build (knowing it won’t compile) so other modules have a chance to successfully build. Once we have the patches in place for the build to succeed, we can decide which branch(es) we want it to land in. How does that sound? -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
