----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26695/#review56713 -----------------------------------------------------------
samza-core/src/main/scala/org/apache/samza/config/ShellCommandConfig.scala <https://reviews.apache.org/r/26695/#comment97110> "even the" -> "even when the" samza-kafka/src/main/scala/org/apache/samza/util/KafkaLog4jAppender.scala <https://reviews.apache.org/r/26695/#comment97095> License. Could we put this in the org.apache.samza.logging.log4j package space, the same way we have the jmx appender? samza-kafka/src/main/scala/org/apache/samza/util/KafkaLog4jAppender.scala <https://reviews.apache.org/r/26695/#comment97105> Get class name programmatically (Class[StringEncoder].getCanonicalName). samza-kafka/src/main/scala/org/apache/samza/util/KafkaLog4jAppender.scala <https://reviews.apache.org/r/26695/#comment97106> Get class name programmatically (Class[TaskIdPartitioner].getCanonicalName). samza-kafka/src/main/scala/org/apache/samza/util/KafkaLog4jAppender.scala <https://reviews.apache.org/r/26695/#comment97107> Nit: Whitespace. samza-kafka/src/main/scala/org/apache/samza/util/KafkaLog4jAppender.scala <https://reviews.apache.org/r/26695/#comment97108> Nit: Whitespace. samza-kafka/src/main/scala/org/apache/samza/util/KafkaLog4jAppender.scala <https://reviews.apache.org/r/26695/#comment97109> Define defaults in object object, and re-use here and in constructor above. samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala <https://reviews.apache.org/r/26695/#comment97103> Should do validation here to make sure that the topic has the expected partition number and replication factor. See KafkaCheckpointManager.create and KafkaCheckpointManager.validate. Should consolidate that code here. samza-kafka/src/main/scala/org/apache/samza/util/TaskIdPartitioner.scala <https://reviews.apache.org/r/26695/#comment97093> License. Do we need this? I think Kafka's default partitioner (https://github.com/apache/kafka/blob/trunk/core/src/main/scala/kafka/producer/DefaultPartitioner.scala) will take care of this for us. samza-kafka/src/main/scala/org/apache/samza/util/TaskIdPartitioner.scala <https://reviews.apache.org/r/26695/#comment97094> Partitioner[space]{ samza-kafka/src/test/scala/org/apache/samza/util/TestKafkaUtil.scala <https://reviews.apache.org/r/26695/#comment97100> License. - Chris Riccomini On Oct. 15, 2014, 4:49 p.m., Yan Fang wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26695/ > ----------------------------------------------------------- > > (Updated Oct. 15, 2014, 4:49 p.m.) > > > Review request for samza. > > > Bugs: SAMZA-310 > https://issues.apache.org/jira/browse/SAMZA-310 > > > Repository: samza > > > Description > ------- > > Publish the logs to the Kafka topic > > > Diffs > ----- > > docs/learn/documentation/versioned/jobs/logging.md 6739740 > samza-api/src/main/java/org/apache/samza/job/CommandBuilder.java f510ce5 > samza-core/src/main/scala/org/apache/samza/config/ShellCommandConfig.scala > 69cea2c > samza-core/src/main/scala/org/apache/samza/job/ShellCommandBuilder.scala > 7519117 > samza-kafka/src/main/scala/org/apache/samza/util/KafkaLog4jAppender.scala > PRE-CREATION > samza-kafka/src/main/scala/org/apache/samza/util/KafkaUtil.scala d660b91 > samza-kafka/src/main/scala/org/apache/samza/util/TaskIdPartitioner.scala > PRE-CREATION > > samza-kafka/src/test/scala/org/apache/samza/util/TestKafkaLog4jAppender.scala > PRE-CREATION > samza-kafka/src/test/scala/org/apache/samza/util/TestKafkaUtil.scala > PRE-CREATION > > samza-yarn/src/main/scala/org/apache/samza/job/yarn/SamzaAppMasterTaskManager.scala > 5b6cc81 > > Diff: https://reviews.apache.org/r/26695/diff/ > > > Testing > ------- > > > Thanks, > > Yan Fang > >
