Thanks Hari, no other better solution now, we only can document in source code to make it clearly. Wish you can agree with the modification for that generated code
2013/2/5 Hari Shreedharan <[email protected]> > Thanks Denny. If you don't have an idea, I will document that in the > pom.xml of the scribe source, and in the ScribeSource.java file, so the > next time thrift is upgraded - the developer can manually change the string > to a ByteBuffer. Does that make sense to you? > > > Hari > > -- > Hari Shreedharan > > > On Tuesday, February 5, 2013 at 12:08 AM, Denny Ye wrote: > > > OK, I'm not sure which is better method to keep the effective code during > > upgrading. > > I will post it if I have a good idea. > > > > > > 2013/2/5 Hari Shreedharan <[email protected] (mailto: > [email protected])> > > > > > OK, no problem. I modified the Scribe Source to simply pick up the > String > > > and convert it to a byte array. Unfortunately, since we need to > maintain > > > the generated code even in between Thrift upgrades, we should probably > not > > > edit the generated code by hand. I am actually OK with modifying that > class > > > to use ByteBuffer, but just document that when we do a Thrift upgrade, > we > > > must change the LogEntry class manually - this is not a good idea, but > if > > > you think that makes it effective and gives better performance I won't > > > complain. Do you think you could do this using a script > (find/replace), so > > > we can add ant-run plugin to the pom.xml to handle this case? > > > > > > > > > Thanks > > > Hari > > > > > > -- > > > Hari Shreedharan > > > > > > > > > On Monday, February 4, 2013 at 11:42 PM, Denny Ye wrote: > > > > > > > hi Hari, the original version of generated code from that Thrift > template > > > > has only support message with 'String' type, I think it's ineffective > > > > > > > > > > that > > > > deserialized from byte stream to String again. That's CPU sensitive > work > > > > and useless in progress of data flow. Thus, I modified code of > LogEntry > > > > > > > > > > to > > > > support byte array incoming from socket except deserializing. > > > > > > > > What should I do next? > > > > > > > > > > > > 2013/2/5 Hari Shreedharan <[email protected] (mailto: > [email protected]) (mailto: > > > [email protected] (mailto:[email protected]))> > > > > > > > > > Denny, > > > > > > > > > > I modified the file's namespace and the final file looks like this: > > > > > > > > > > namespace java org.apache.flume.source.scribe > > > > > > > > > > enum ResultCode > > > > > { > > > > > OK, > > > > > TRY_LATER > > > > > } > > > > > > > > > > struct LogEntry > > > > > { > > > > > 1: string category, > > > > > 2: string message > > > > > } > > > > > > > > > > service Scribe > > > > > { > > > > > ResultCode Log(1: list<LogEntry> messages); > > > > > } > > > > > > > > > > > > > > > > > > > > But it looks like the ScribeSource.java (line 154) expects > > > > > LogEntry.message to be "binary" and not "string." Since there are > no > > > > > > > > > > > > > > > > > > > unit > > > > > tests, I cannot verify that making message binary will not break > > > > > > > > > > something. > > > > > Could you give me some fix for this? > > > > > > > > > > > > > > > Thanks > > > > > Hari > > > > > > > > > > -- > > > > > Hari Shreedharan > > > > > > > > > > > > > > > On Monday, February 4, 2013 at 8:05 PM, Hari Shreedharan wrote: > > > > > > > > > > > Hi Denny, Juhani, > > > > > > > > > > > > Thanks! I will add this file to the compileThrift profile, so we > can > > > > > update the thrift generated code when we upgrade thrift. > > > > > > > > > > > > Thanks! > > > > > > Hari > > > > > > > > > > > > -- > > > > > > Hari Shreedharan > > > > > > > > > > > > > > > > > > On Monday, February 4, 2013 at 6:46 PM, Denny Ye wrote: > > > > > > > > > > > > > Sorry for late response > > > > > > > > > > > > > > Here is the scribe.thrift file : > > > > > > > ************************* file separator ********************* > > > > > > > namespace java scribe.thrift > > > > > > > > > > > > > > enum ResultCode > > > > > > > { > > > > > > > OK, > > > > > > > TRY_LATER > > > > > > > } > > > > > > > > > > > > > > struct LogEntry > > > > > > > { > > > > > > > 1: string category, > > > > > > > 2: string message > > > > > > > } > > > > > > > > > > > > > > service scribe > > > > > > > { > > > > > > > ResultCode Log(1: list<LogEntry> messages); > > > > > > > } > > > > > > > ************************* file separator ********************* > > > > > > > > > > > > > > To Juhani, I haven't use the facebook template, there have so > many > > > > > > > additional interfaces that I don't like it. > > > > > > > > > > > > > > > > > > > > > > > > > > > > 2013/2/5 Juhani Connolly <[email protected](mailto: > [email protected])(mailto: > > > [email protected] (mailto: > [email protected])) (mailto: > > > > > [email protected] (mailto: > [email protected]) (mailto: > > > > > > > > > > [email protected] (mailto: > [email protected])))> > > > > > > > > > > > > > > > This is the file we want: > > > https://github.com/facebook/**scribe/blob/master/if/scribe.**thrift< > > > > > https://github.com/facebook/scribe/blob/master/if/scribe.thrift> > > > > > > > > > > > > > > > > It's apache 2 licensed but also has a facebooke copyright > > > header. I > > > > > think > > > > > > > > this is why we didn't originally include it. > > > > > > > > > > > > > > > > > > > > > > > > On 02/03/2013 09:14 AM, Hari Shreedharan wrote: > > > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > > > The scribe source has some thrift generated code committed, > > > but no > > > > > > > > > ".thrift" file. This makes upgrading to a new version of > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > libthrift > > > > > > > > > impossible. > > > > > > > > > > > > > > > > > > I believe Denny Ye contributed this code, so Denny, it'd be > > > nice > > > > > if you > > > > > > > > > could submit the ".thrift" file as soon as possible. > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > > Hari > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
