> On Sept. 10, 2012, 6:47 p.m., Mike Percy wrote: > > I'm not sure this is a great idea from a maintainability perspective. > > Brock, do you think this is necessary? > > Brock Noland wrote: > Why is it a problem from a maintainability perspective? If someone > changes the .proto file they need to regenerate the .java file and check the > changes in. That is a rare event and they'd need to actually generate the > source so they can use the change. > > The protoc compiler is old on the jenkins build slaves so it generates > code which does not compile with recent versions of the protocol buffers java > library. You can see the build failing here > https://builds.apache.org/job/flume-trunk/296/console (portion of the message > below). > > {noformat} > [ERROR] > /home/jenkins/jenkins-slave/workspace/flume-trunk/flume-ng-channels/flume-file-channel/target/generated-sources/java/org/apache/flume/chanel/file/proto/ProtosFactory.java:[11,22] > org.apache.flume.chanel.file.proto.ProtosFactory.Checkpoint is not abstract > and does not override abstract method > newBuilderForType(com.google.protobuf.GeneratedMessage.BuilderParent) in > com.google.protobuf.GeneratedMessage > {noformat} > >
FWIW, I actually came up with idea after re-reading this email chain: http://mail-archives.apache.org/mod_mbox/flume-dev/201208.mbox/%[email protected]%3E Where you stated: "However, this adds a build-time dependency to (a specific version of) Thrift, so I'd be inclined to check in the generated code and make it possible to regenerate it via Maven, like the legacy stuff is supposed to work." - Brock ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6985/#review11264 ----------------------------------------------------------- On Sept. 10, 2012, 8:10 p.m., Brock Noland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6985/ > ----------------------------------------------------------- > > (Updated Sept. 10, 2012, 8:10 p.m.) > > > Review request for Flume. > > > Description > ------- > > Instead of generating ProtoFactory each time we run maven, the class can be > generated using: > > mvn generate-sources -Pcompile-proto > > This is because we cannot depend on user/build machines having an updated > protoc compiler. > > > This addresses bug FLUME-1554. > https://issues.apache.org/jira/browse/FLUME-1554 > > > Diffs > ----- > > flume-ng-channels/flume-file-channel/pom.xml 639195e > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Commit.java > 7d57bb8 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/EventQueueBackingStoreFileV3.java > c766d09 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java > 414d6d9 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Put.java > 884ebde > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Rollback.java > 1a8ddff > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Take.java > cfbe0c9 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/TransactionEventRecord.java > f78147f > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/proto/ProtosFactory.java > PRE-CREATION > flume-ng-channels/flume-file-channel/src/main/proto/filechannel.proto > 59dc8db > > Diff: https://reviews.apache.org/r/6985/diff/ > > > Testing > ------- > > unit tests pass > > > Thanks, > > Brock Noland > >
