samza-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chris Riccomini" <criccom...@apache.org>
Subject Re: Review Request 28035: SAMZA-310-1
Date Fri, 14 Nov 2014 20:07:26 GMT


> On Nov. 14, 2014, 7:05 p.m., Chris Riccomini wrote:
> > Looks like this was coded against master. Are you planning to backport to 0.8.0
branch?
> > 
> > I think we should either keep this module scala-free, or should add scala as a first-class
citizen (i.e. implement the config code using real Scala). I'm not a big fan of bleeding Scala
APIs into Java (e.g. Option).
> > 
> > If we keep it scala free, we'll have to duplicate a couple of config names, but
I think that's an OK trade-off. SAMZA-40 is going to fix this stuff (Config with Java as a
first class citizen) anyway.
> 
> Yan Fang wrote:
>     "coded against master"
>     -- yes, I realized it this morning :(. Let's see if I can get it done today, if not,
I am ok with only committing to master. 
>     
>     yes, i would prefere to make it scala-free because samza-log4j itself is pure-java.
>     
>     But in the test case, some Kafka APIs (such as "Utils.rm(server.config().logDirs())")
only accept Scala time, is it ok to add scala in the test scope?

Totally. testCompile for Scala is fine with me.


> On Nov. 14, 2014, 7:05 p.m., Chris Riccomini wrote:
> > samza-log4j/src/main/java/org/apache/samza/logging/log4j/SystemProducerAppender.java,
line 54
> > <https://reviews.apache.org/r/28035/diff/1/?file=763402#file763402line54>
> >
> >     This is pretty confusing. Since we're using a string, can we just have it default
to "application-master", and use "samza-container-%d" for containers?
> 
> Yan Fang wrote:
>     yes, I think we can. Then it will be the system's responsiblity to partition based
on the key. For example, in the kafka, users need to write a customized partition class to
deal with "application-master". Another option is that, we can have partitionKey and Key.
Use "0, 1, ..." for partitionKey and "application-master" for key.

In Kafka's case, shouldn't it just work as-is with "application-master"? I think the default
partitioner just hashCode's the object, then mod's it to the partition size.


- Chris


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28035/#review61461
-----------------------------------------------------------


On Nov. 14, 2014, 11:40 a.m., Yan Fang wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28035/
> -----------------------------------------------------------
> 
> (Updated Nov. 14, 2014, 11:40 a.m.)
> 
> 
> Review request for samza.
> 
> 
> Bugs: SAMZA-310
>     https://issues.apache.org/jira/browse/SAMZA-310
> 
> 
> Repository: samza
> 
> 
> Description
> -------
> 
> Added a log4j appender using SystemProducer
> Added a log4jConfig class to help get log4j specific config
> Unit test code
> 
> 
> Diffs
> -----
> 
>   build.gradle 828bce9 
>   samza-log4j/src/main/java/org/apache/samza/config/Log4jSystemConfig.java PRE-CREATION

>   samza-log4j/src/main/java/org/apache/samza/logging/log4j/SystemProducerAppender.java
PRE-CREATION 
>   samza-log4j/src/test/java/org/apache/samza/logging/log4j/TestSystemProducerAppender.java
PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/28035/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Yan Fang
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message