> On July 1, 2014, 1:55 p.m., Kevin Sweeney wrote:
> > src/main/python/apache/aurora/client/cli/context.py, lines 63-64
> > <https://reviews.apache.org/r/23199/diff/1/?file=621349#file621349line63>
> >
> >     This doesn't play nicely with the include() directive, which may load 
> > other config files from elsewhere. It also introduces a race (you open and 
> > close the config file here, then the loader presumably does as well meaning 
> > that the file contents could change after you've logged them). I wonder if 
> > this logic would be better pushed down to the loader.

(1) Race condition really isn't an issue here. If a user is modifying a config 
file while aurora is running, the results will (at best) be flaky. This doesn't 
make that any worse.

(2) I'd like to put it in the loader, but it's not feasible. The loader is, 
ultimately, inside of pystachio (via a complex route: get_config -> 
AnnotatedAuroraConfig.load -> AuroraConfig.load -> AuroraConfigLoader.load -> 
AuroraConfigLoader.load_raw -> PystachioConfig.__apply_)

(3) This doesn't follow include semantics - but it's not trying to solve that 
problem. The way that the aurora docs read, the config file should be at the 
path specified on the command-line. If it's not, it should be an error. For 
this level of logging, I'm not even trying to resolve includes - just simply 
dumping file contents.


> On July 1, 2014, 1:55 p.m., Kevin Sweeney wrote:
> > src/main/python/apache/aurora/client/cli/context.py, line 64
> > <https://reviews.apache.org/r/23199/diff/1/?file=621349#file621349line64>
> >
> >     Add a constant for the TRANSCRIPT loglevel somewhere?

There is, but using it here would create a circular dependency.


- Mark


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


On July 1, 2014, 11:03 a.m., Mark Chu-Carroll wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/23199/
> -----------------------------------------------------------
> 
> (Updated July 1, 2014, 11:03 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: aurora-567
>     https://issues.apache.org/jira/browse/aurora-567
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Have client commands that load config files print the contents of the loaded 
> config file to 
> log at level TRANSCRIPT (aka INFO+1).
> 
> 
> Diffs
> -----
> 
>   src/main/python/apache/aurora/client/cli/context.py 
> facd52c14e330c35889cec0292bb380cecff9641 
>   src/test/python/apache/aurora/client/cli/test_logging.py 
> b3c3b8deaa8961251a1be8121659e742b00f6df2 
> 
> Diff: https://reviews.apache.org/r/23199/diff/
> 
> 
> Testing
> -------
> 
> Added unit test of new functionality, ran all tests.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>

Reply via email to