> On May 21, 2015, 6:45 p.m., Navina Ramesh wrote: > > samza-core/src/main/java/org/apache/samza/config/JavaStorageConfig.java, > > line 28 > > <https://reviews.apache.org/r/33419/diff/3/?file=965787#file965787line28> > > > > Is "moving away from scala based configs" a motivation for all the > > "Java" prefixed classes? > > > > If the plan is to refactor configs everywhere with Java based configs, > > then we should probably extend these classes from > > org.apache.samza.config.MapConfig. It provides some convenient methods > > because it is backed by a Map. Just curious if you explored that option.
This makes sense. I omitted this option. Thanks. Using MapConfig now. Also added unit test to verify that. > On May 21, 2015, 6:45 p.m., Navina Ramesh wrote: > > samza-core/src/main/java/org/apache/samza/config/JavaSystemConfig.java, > > line 33 > > <https://reviews.apache.org/r/33419/diff/3/?file=965788#file965788line33> > > > > Why is Config "protected" here and "private" in JavaStorageConfig? fixed in above change. > On May 21, 2015, 6:45 p.m., Navina Ramesh wrote: > > samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala, > > line 39 > > <https://reviews.apache.org/r/33419/diff/3/?file=965791#file965791line39> > > > > why not sanitize the entire file name, instead of just the task name? > > Personally, I think we should follow a standard convention for > > sanitizing names, irrespective of whether it is storename or taskname. Just > > my two cents. aggreed. - Yan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33419/#review84612 ----------------------------------------------------------- On June 21, 2015, 6:10 a.m., Yan Fang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33419/ > ----------------------------------------------------------- > > (Updated June 21, 2015, 6:10 a.m.) > > > Review request for samza. > > > Bugs: SAMZA-625 > https://issues.apache.org/jira/browse/SAMZA-625 > > > Repository: samza > > > Description > ------- > > Implemented in Java. > > * modified build.gradle to have the gradle compile scala first. Because some > jave code has dependencies to Scala code > * change the state store name by removing the space ( in TaskManager ) > * add scala java conversion method in Util because some classes only accept > scala map > * add java version of some configs > * remove duplicated config in samza-log4j > * add StorageRevoery class, which does most of the recoverying job. The logic > mimics what happens in SamzaContainer. > * add StateStorageTool, for the commandline usage > * unit tests > * docs > > > Diffs > ----- > > checkstyle/import-control.xml 3374f0c > docs/learn/documentation/versioned/container/state-management.md 79067bb > samza-core/src/main/java/org/apache/samza/config/JavaStorageConfig.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/config/JavaSystemConfig.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/storage/StateStorageTool.java > PRE-CREATION > samza-core/src/main/java/org/apache/samza/storage/StorageRecovery.java > PRE-CREATION > samza-core/src/main/scala/org/apache/samza/storage/TaskStorageManager.scala > aeba61a > samza-core/src/main/scala/org/apache/samza/util/Util.scala 2feb65b > samza-core/src/test/java/org/apache/samza/config/TestJavaStorageConfig.java > PRE-CREATION > samza-core/src/test/java/org/apache/samza/config/TestJavaSystemConfig.java > PRE-CREATION > samza-core/src/test/java/org/apache/samza/storage/MockStorageEngine.java > PRE-CREATION > > samza-core/src/test/java/org/apache/samza/storage/MockStorageEngineFactory.java > PRE-CREATION > samza-core/src/test/java/org/apache/samza/storage/MockSystemConsumer.java > PRE-CREATION > samza-core/src/test/java/org/apache/samza/storage/MockSystemFactory.java > PRE-CREATION > samza-core/src/test/java/org/apache/samza/storage/TestStorageRecovery.java > PRE-CREATION > samza-log4j/src/main/java/org/apache/samza/config/Log4jSystemConfig.java > d5e24f2 > samza-shell/src/main/bash/state-storage-tool.sh PRE-CREATION > > Diff: https://reviews.apache.org/r/33419/diff/ > > > Testing > ------- > > tested with multiple partitions and multiple stores recovery. > > > Thanks, > > Yan Fang > >
