> On Oct. 27, 2014, 11 p.m., Chris Riccomini wrote: > > samza-kafka/src/main/scala/org/apache/samza/util/CheckpointMigrationTool.scala, > > line 191 > > <https://reviews.apache.org/r/27246/diff/1/?file=734567#file734567line191> > > > > Would rather re-use KafkaCheckpointManagerFactory's method. We'll have > > to move it to the `object`, and make it, but seems better than duplicating > > logic.
Yeah. I didn't move it to object because it was defined as "private" and I didn't understand why. I will move it to object as it makes more sense. > On Oct. 27, 2014, 11 p.m., Chris Riccomini wrote: > > samza-kafka/src/main/scala/org/apache/samza/util/CheckpointMigrationTool.scala, > > line 131 > > <https://reviews.apache.org/r/27246/diff/1/?file=734567#file734567line131> > > > > It seems dangerous to default to "None" if we can't find a checkpoint > > for the SSP. I think we should throw an exception in such a case. > > > > As it is now, getOrElse(None).toString will put something like SSP1 -> > > "None", which will cause an exception when the job starts, since it'll try > > to do "None".toInt I wasn't sure how to handle this actually. A good way for the tool to handle the "null" scenario can be to read the consumer offset.reset settings. I am not sure if we should go this route or just throw an exception and let the user deal with fixing it. > On Oct. 27, 2014, 11 p.m., Chris Riccomini wrote: > > samza-kafka/src/main/scala/org/apache/samza/util/CheckpointMigrationTool.scala, > > line 67 > > <https://reviews.apache.org/r/27246/diff/1/?file=734567#file734567line67> > > > > Seems to be never used, other than logCheckpoint, which is also never > > used. Yeah. Looks like SSP_PATTERN, LOG_TYPE and logCheckpoint are unnecessary. Removing them. On Oct. 27, 2014, 11 p.m., Navina Ramesh wrote: > > Also, I want to confirm--you're developing this off of the 0.8.0 branch, > > not master, right? I am developing off master branch :( Should I develop this off the 0.8.0 branch ? - Navina ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27246/#review58703 ----------------------------------------------------------- On Oct. 27, 2014, 9:40 p.m., Navina Ramesh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27246/ > ----------------------------------------------------------- > > (Updated Oct. 27, 2014, 9:40 p.m.) > > > Review request for samza. > > > Repository: samza > > > Description > ------- > > Adding checkpoint migration tool LISAMZA-594 > > > Diffs > ----- > > build.gradle 828bce9913db00161971607e4c9ac19c63cecb95 > > samza-kafka/src/main/scala/org/apache/samza/util/CheckpointMigrationTool.scala > PRE-CREATION > samza-shell/src/main/bash/run-migration-tool.sh PRE-CREATION > > Diff: https://reviews.apache.org/r/27246/diff/ > > > Testing > ------- > > Created a test job "samza-migration-mp" using 0.7 version. Stopped the job > and ran the migration tool. > > > Thanks, > > Navina Ramesh > >
