Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/#review146232 --- Ship it! Ship It! - Yi Pan (Data Infrastructure) On Aug. 18,

Re: Review Request 48356: RFC: Samza as a library

> On July 15, 2016, 12:12 a.m., Yi Pan (Data Infrastructure) wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java, > > line 31 > > > > > > Just wonder, can we have an interface class

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/ --- (Updated Aug. 18, 2016, 5:39 p.m.) Review request for samza, Chris Pettitt and

Re: Review Request 48356: RFC: Samza as a library

> On Aug. 17, 2016, 8:36 a.m., Yi Pan (Data Infrastructure) wrote: > > A quick question: w/ the recent discussion, are you planning to remove the > > ConfigBuilder interface and add a StreamProcessorFactory? Yes Yi. As we discussed online, I think we should commit this RB (as is) in order to u

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/#review145967 --- A quick question: w/ the recent discussion, are you planning to re

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/ --- (Updated Aug. 12, 2016, 3:29 a.m.) Review request for samza, Chris Pettitt and

Re: Review Request 48356: RFC: Samza as a library

> On July 15, 2016, 12:12 a.m., Yi Pan (Data Infrastructure) wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java, > > line 107 > > > > > > How would it work w/ RegexConfigRewriter?

Re: Review Request 48356: RFC: Samza as a library

> On July 15, 2016, 12:12 a.m., Yi Pan (Data Infrastructure) wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java, > > line 107 > > > > > > How would it work w/ RegexConfigRewriter?

Re: Review Request 48356: RFC: Samza as a library

> On July 15, 2016, 12:12 a.m., Yi Pan (Data Infrastructure) wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java, > > line 58 > > > > > > Here it seems like that you are mandating an

Re: Review Request 48356: RFC: Samza as a library

> On July 15, 2016, 12:12 a.m., Yi Pan (Data Infrastructure) wrote: > > samza-core/src/main/java/org/apache/samza/processor/StreamProcessor.java, > > line 130 > > > > > > Not sure whether we want to keep JmxServer l

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/ --- (Updated Aug. 11, 2016, 1:23 a.m.) Review request for samza, Chris Pettitt and

Re: Review Request 48356: RFC: Samza as a library

> On July 15, 2016, 12:12 a.m., Yi Pan (Data Infrastructure) wrote: > > samza-core/src/main/java/org/apache/samza/processor/StreamProcessor.java, > > line 130 > > > > > > Not sure whether we want to keep JmxServer l

Re: Review Request 48356: RFC: Samza as a library

> On July 15, 2016, 12:12 a.m., Yi Pan (Data Infrastructure) wrote: > > samza-core/src/main/java/org/apache/samza/standalone/StandaloneJobCoordinator.java, > > line 70 > > > > > > I don't fully understand the "unfort

Re: Review Request 48356: RFC: Samza as a library

> On July 15, 2016, 12:12 a.m., Yi Pan (Data Infrastructure) wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/CheckpointConfig.java, > > line 24 > > > > > > So, what's the plan to make customized Ch

Re: Review Request 48356: RFC: Samza as a library

> On June 27, 2016, 6:53 p.m., Chris Pettitt wrote: > > samza-core/src/main/java/org/apache/samza/standalone/StandaloneJobCoordinator.java, > > lines 92-94 > > > > > > How is this used? It seems to be write only? >

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/#review142183 --- Thanks for pulling it off! I did the first round of review. If the

Re: Review Request 48356: RFC: Samza as a library

> On June 27, 2016, 6:53 p.m., Chris Pettitt wrote: > > samza-core/src/main/java/org/apache/samza/standalone/StandaloneJobCoordinator.java, > > line 44 > > > > > > If this is write-once I would move this to the const

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/ --- (Updated July 13, 2016, 9:58 p.m.) Review request for samza, Chris Pettitt and

Re: Review Request 48356: RFC: Samza as a library

> On July 12, 2016, 12:15 a.m., Fred Ji wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java, > > line 92 > > > > > > Maybe if(!StringUtils.isBlank())? Since we would like to have a v

Re: Review Request 48356: RFC: Samza as a library

> On July 12, 2016, 12:15 a.m., Fred Ji wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java, > > line 92 > > > > > > Maybe if(!StringUtils.isBlank())? Since we would like to have a v

Re: Review Request 48356: RFC: Samza as a library

> On July 12, 2016, 12:15 a.m., Fred Ji wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java, > > line 92 > > > > > > Maybe if(!StringUtils.isBlank())? Since we would like to have a v

Re: Review Request 48356: RFC: Samza as a library

> On July 12, 2016, 12:15 a.m., Fred Ji wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java, > > line 92 > > > > > > Maybe if(!StringUtils.isBlank())? Since we would like to have a v

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/ --- (Updated July 12, 2016, 10:02 p.m.) Review request for samza, Chris Pettitt and

Re: Review Request 48356: RFC: Samza as a library

> On July 12, 2016, 12:15 a.m., Fred Ji wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/SerdeConfig.java, > > line 43 > > > > > > Could serdeAlias be null accidentally? If so, it would be better to

Re: Review Request 48356: RFC: Samza as a library

> On July 12, 2016, 12:15 a.m., Fred Ji wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java, > > line 92 > > > > > > Maybe if(!StringUtils.isBlank())? Since we would like to have a v

Re: Review Request 48356: RFC: Samza as a library

> On July 12, 2016, 6:43 p.m., Chris Pettitt wrote: > > samza-core/src/main/java/org/apache/samza/processor/StreamProcessor.java, > > lines 174-176 > > > > > > Don't we need to synchronize these? Are we guaranteed

Re: Review Request 48356: RFC: Samza as a library

> On July 12, 2016, 6:43 p.m., Chris Pettitt wrote: > > samza-core/src/main/java/org/apache/samza/processor/StreamProcessor.java, > > lines 174-176 > > > > > > Don't we need to synchronize these? Are we guaranteed

Re: Review Request 48356: RFC: Samza as a library

> On June 27, 2016, 6:53 p.m., Chris Pettitt wrote: > > A few more thoughts below. > > > > Still not a fan of the direction we're going with the config. I know it is > > status quo, but it further locks us into a limited model. One other benefit > > of the Offspring way of doing config that oc

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/ --- (Updated July 12, 2016, 6:45 p.m.) Review request for samza, Chris Pettitt and

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/#review141942 --- Fix it, then Ship it! Noticed one issue while responding to you

Re: Review Request 48356: RFC: Samza as a library

> On July 12, 2016, 12:15 a.m., Fred Ji wrote: > > samza-core/src/main/java/org/apache/samza/config/TaskConfigJava.java, line > > 131 > > > > > > [INFO Question] Just curious, Is there a required code convention for

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/#review141799 --- Thanks a lot for the RB! I only have a few INFO questions to learn

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/ --- (Updated July 12, 2016, midnight) Review request for samza, Chris Pettitt and Y

Re: Review Request 48356: RFC: Samza as a library

> On June 27, 2016, 6:53 p.m., Chris Pettitt wrote: > > samza-core/src/main/java/org/apache/samza/processor/StreamProcessor.java, > > lines 125-126 > > > > > > Don't we need to stop the container directly here? shut

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/ --- (Updated July 11, 2016, 10:47 p.m.) Review request for samza, Chris Pettitt and

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/ --- (Updated July 11, 2016, 10:27 p.m.) Review request for samza, Chris Pettitt and

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/ --- (Updated June 28, 2016, 11:49 p.m.) Review request for samza, Chris Pettitt and

Re: Review Request 48356: RFC: Samza as a library

> On June 27, 2016, 6:53 p.m., Chris Pettitt wrote: > > samza-core/src/main/java/org/apache/samza/standalone/StandaloneJobCoordinator.java, > > line 44 > > > > > > If this is write-once I would move this to the const

Re: Review Request 48356: RFC: Samza as a library

> On June 27, 2016, 6:53 p.m., Chris Pettitt wrote: > > samza-core/src/main/java/org/apache/samza/processor/StreamProcessor.java, > > lines 125-126 > > > > > > Don't we need to stop the container directly here? shut

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/ --- (Updated June 28, 2016, 8:20 p.m.) Review request for samza, Chris Pettitt and

Re: Review Request 48356: RFC: Samza as a library

> On June 27, 2016, 6:53 p.m., Chris Pettitt wrote: > > samza-core/src/main/java/org/apache/samza/processor/StreamProcessor.java, > > lines 125-126 > > > > > > Don't we need to stop the container directly here? shut

Re: Review Request 48356: RFC: Samza as a library

> On June 27, 2016, 6:53 p.m., Chris Pettitt wrote: > > samza-core/src/main/java/org/apache/samza/processor/StreamProcessor.java, > > lines 125-126 > > > > > > Don't we need to stop the container directly here? shut

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/#review139626 --- A few more thoughts below. Still not a fan of the direction we're

Re: Review Request 48356: RFC: Samza as a library

> On June 9, 2016, 7:15 p.m., Chris Pettitt wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java, > > lines 33-39 > > > > > > Do these need to public? I checked JOB_NAME only, but it

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/ --- (Updated June 23, 2016, 1:13 a.m.) Review request for samza and Chris Pettitt.

Re: Review Request 48356: RFC: Samza as a library

> On June 9, 2016, 7:15 p.m., Chris Pettitt wrote: > > samza-core/src/main/java/org/apache/samza/configbuilder/ConfigBuilder.java, > > lines 42-53 > > > > > > Can these be private vs. package private? > > Navina Ram

Re: Review Request 48356: RFC: Samza as a library

> On June 9, 2016, 7:15 p.m., Chris Pettitt wrote: > > samza-core/src/main/scala/org/apache/samza/container/SamzaContainer.scala, > > lines 620-622 > > > > > > I hope this is temporary. If not, if would be nicer to

Re: Review Request 48356: RFC: Samza as a library

> On June 9, 2016, 7:15 p.m., Chris Pettitt wrote: > > Thoughts below. A more general thought: the builders don't seem to be > > providing a lot of value and they're constraining you to a bad API (string > > based, untyped). I'd strongly suggest that we use typed builders and map > > this back

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/#review136871 --- Thoughts below. A more general thought: the builders don't seem to

Re: Review Request 48356: RFC: Samza as a library

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48356/ --- (Updated June 8, 2016, 9:59 p.m.) Review request for samza and Chris Pettitt.