> On April 17, 2015, 9:23 p.m., Jakob Homan wrote: > > samza-api/src/main/java/org/apache/samza/config/EnvironmentConfigRewriter.java, > > line 41 > > <https://reviews.apache.org/r/33305/diff/1/?file=933124#file933124line41> > > > > It may be worth being chatty here in terms of the new config properties > > that are being introduced. Better to have some extra info in the startup > > log and not need it than no info and need it.
Yes, this was the dead code below but I didn't want to add the slf4j dependency to samza-api but as it's now in core it's already there. So will log what's being imported / over-ridden. > On April 17, 2015, 9:23 p.m., Jakob Homan wrote: > > samza-api/src/main/java/org/apache/samza/config/EnvironmentConfigRewriter.java, > > line 49 > > <https://reviews.apache.org/r/33305/diff/1/?file=933124#file933124line49> > > > > Can we warn if we are overriding a previously defined key? I can see > > the potential for difficult-to-debug issues if keys are silently overridden. Yes, I've added this to the logging too. - Dan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33305/#review80525 ----------------------------------------------------------- On April 17, 2015, 1:12 p.m., Dan Harvey wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/33305/ > ----------------------------------------------------------- > > (Updated April 17, 2015, 1:12 p.m.) > > > Review request for samza. > > > Repository: samza > > > Description > ------- > > [SAMZA-655] Add EnvironmentConfigRewriter to samza-api. > > > Diffs > ----- > > > samza-api/src/main/java/org/apache/samza/config/EnvironmentConfigRewriter.java > PRE-CREATION > > samza-api/src/test/java/org/apache/samza/config/EnvironmentConfigRewriterTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/33305/diff/ > > > Testing > ------- > > > Thanks, > > Dan Harvey > >
