----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35241/#review90271 -----------------------------------------------------------
Lgtm! samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamWriter.java (line 49) <https://reviews.apache.org/r/35241/#comment145198> Can you get rid of the factory instance and just do new CoordinatoryStreamSystemFactory().getCoordinatorStreamSystemProducer(... ? samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamWriter.java (line 73) <https://reviews.apache.org/r/35241/#comment145196> typo -> "set-confing" samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamWriter.java (line 81) <https://reviews.apache.org/r/35241/#comment145197> We should validate the config keys & values (if possible) in the future. Can you please add this as TODO here? samza-core/src/main/scala/org/apache/samza/coordinator/stream/CoordinatorStreamWriterCommandLine.scala (line 56) <https://reviews.apache.org/r/35241/#comment145199> Simpler to follow a pattern like: {code} if(option.has(messageKey)) return option.valueOf(meesageKey) else return null {code} This way you can also avoid using "var". samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamSystemFactory.java (line 26) <https://reviews.apache.org/r/35241/#comment143275> Can you revert this import? We prefer to import only specific class that are being used as opposed to the entire package. samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamSystemFactory.java (line 107) <https://reviews.apache.org/r/35241/#comment145210> start & stop inverse of each other. Do you really need both the flags? :) samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamWriter.java (line 66) <https://reviews.apache.org/r/35241/#comment145212> Is it possible to simplify Lines 59-65? Looks like you are doing a null check in Line 61 and then, creating another handle for the field in Line 64. - Navina Ramesh On June 29, 2015, 11:08 p.m., Shadi A. Noghabi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35241/ > ----------------------------------------------------------- > > (Updated June 29, 2015, 11:08 p.m.) > > > Review request for samza, Yi Pan (Data Infrastructure), Navina Ramesh, and > Naveen Somasundaram. > > > Repository: samza > > > Description > ------- > > In order to be able to change configurations while a job is running, a tool > for writing a message to the coordinator stream is needed. This code targets > creating such a tool that can write messages to the coordinator stream after > the bootstrap of the job. This code is related to the SAMZA-704 JIRA. > > > To run the code use the following command: > > {path to samza deployment}/samza/bin/run-coordinator-stream-writer.sh > --config-factory={config-factory} --config-path={path to config file of a > job} --type={type of the message} --key={[optional] key of the message} > --value={[optional] value of the message} > > > Diffs > ----- > > checkstyle/import-control.xml 3374f0c432e61ac4cda275377604cfd481f0cddf > > samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamMessage.java > 6c1e488d00d8593d59c89b57e673e0b6b90fd7d2 > > samza-core/src/main/java/org/apache/samza/coordinator/stream/CoordinatorStreamWriter.java > PRE-CREATION > > samza-core/src/main/scala/org/apache/samza/coordinator/stream/CoordinatorStreamWriterCommandLine.scala > PRE-CREATION > > samza-core/src/test/java/org/apache/samza/coordinator/stream/MockCoordinatorStreamSystemFactory.java > 647cadb3a4e51bec8204197d77ad35a6b29afcec > > samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamSystemProducer.java > 68e32554c18f443565284b807f43f4a5b05afbce > > samza-core/src/test/java/org/apache/samza/coordinator/stream/TestCoordinatorStreamWriter.java > PRE-CREATION > samza-shell/src/main/bash/run-coordinator-stream-writer.sh PRE-CREATION > > Diff: https://reviews.apache.org/r/35241/diff/ > > > Testing > ------- > > To run the code use the following command: > > {path to samza deployment}/samza/bin/run-coordinator-stream-writer.sh > --config-factory={config-factory} --config-path={path to config file of a > job} --type={type of the message} --key={[optional] key of the message} > --value={[optional] value of the message} > > > Thanks, > > Shadi A. Noghabi > >
