samza-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Dan Harvey" <danharve...@gmail.com>
Subject Re: Review Request 33305: [SAMZA-655] Add EnvironmentConfigRewriter to samza-api.
Date Wed, 22 Apr 2015 14:51:34 GMT


> 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
> 
>


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